This is a nice and easy to understand example of a memory-safety bug that CHERI [1] prevents (among other classes of vulnerabilities). Given that the SYSCTL_PROC() macro installs a pointer to an uint16_t value in the oid_arg1 field, a CHERI pure-capability kernel would construct a capability with bounds set to sizeof(uint16_t) and later the dereference of (int *)oidp->oid_arg1 in sysctl_udp_log_port() would trigger a capability bounds violation.
`sysctl -a` would simply crash on CHERI allowing a developer to catch this without KASAN being involved.
I am not surprised. First, it's a subtle bug. Second, in C/C++. a lot of times you get unlucky when reading uninitialized memory. Basically, the bug does not occur when you test the code on your machine or when you run the automated tests.
Another problem is writing good automated tests is hard and often skipped. Lots of software engineering teams talk about the wonders of automated tests. Unfortunately, many automated tests are not very good and either do not ensure the major functionality works or just do not test some of the code. There are also limits to how much time a software engineer has to test. No one can test everything.
Basically, I am not surprised developers make mistakes and I am not surprised the tests either did not catch this mistake or even did not exist. Software is very hard and software engineers are far from perfect.
- int new_value = *(int *)oidp->oid_arg1;
+ int new_value = *(uint16_t *)oidp->oid_arg1;
Why not just have `uint16_t new_value = ...`?
Ahh, because `new_value` is being given to `sysctl_handle_int(..., &new_value, ...);` which of course expects an `int`. So then it begs the question: if the value is really a `uint16_t`, then why is it being handled through a plain `int`? It smells like there could easily be tons of other memory-safety and/or type confusion problems endemic to the sysctl API.
Well there's the so-called usual arithmetic conversions that will basically convert every uint16_t to an unsigned int. The C and C++ languages do a silent conversion on your back anyways so you might as well make it explicit.
Leaking two random bytes and in some cases just padding bytes to user space is not the end of the world and I don't get why there are so many negative comments blaming Apple for not handing out a handsome bounty for this bug.
It's still a security bug. Often, multiple bugs like this are chained together to create one very nasty exploit. I agree that this bug probably does not deserve a massive payout, but I think $3,000-5,000$ is appropriate.
Is there any reason to assume a conspiracy and drama around the bounty here other that just being bored and cynical? Apple has a well known security bounty program https://security.apple.com/bounty/
kwitaszczyk|1 year ago
`sysctl -a` would simply crash on CHERI allowing a developer to catch this without KASAN being involved.
[1] http://cheri-cpu.org
pjmlp|1 year ago
nixpulvis|1 year ago
0x457|1 year ago
StressedDev|1 year ago
Another problem is writing good automated tests is hard and often skipped. Lots of software engineering teams talk about the wonders of automated tests. Unfortunately, many automated tests are not very good and either do not ensure the major functionality works or just do not test some of the code. There are also limits to how much time a software engineer has to test. No one can test everything.
Basically, I am not surprised developers make mistakes and I am not surprised the tests either did not catch this mistake or even did not exist. Software is very hard and software engineers are far from perfect.
nxobject|1 year ago
ChocolateGod|1 year ago
Correct me if I'm wrong but you get 2 bytes of kernel data (potentially blank padding) and the same two bytes each time?
unknown|1 year ago
[deleted]
inetknght|1 year ago
Ahh, because `new_value` is being given to `sysctl_handle_int(..., &new_value, ...);` which of course expects an `int`. So then it begs the question: if the value is really a `uint16_t`, then why is it being handled through a plain `int`? It smells like there could easily be tons of other memory-safety and/or type confusion problems endemic to the sysctl API.
aritashion|1 year ago
I don't think it begs the question, but it does raise one!
kccqzy|1 year ago
soheil|1 year ago
StressedDev|1 year ago
unknown|1 year ago
[deleted]
rvz|1 year ago
[deleted]
wilg|1 year ago
zx8080|1 year ago
Why would they pay if no profit if pay and they are not forced to pay?
unknown|1 year ago
[deleted]
yapyap|1 year ago
jfasi|1 year ago
https://chatgpt.com/share/6793a2d1-5f84-8006-8e78-16be4d4908...