(no title)
wizeman | 2 years ago
You are probably correct (although it's hard to say for sure that you've considered every single compiler optimization).
But even if you are correct, the point of the parent poster still stands:
> something that triggers undefined behavior in C is not to be dismissed as lightly as the article is doing.
I agree with this.
Notice how long your explanation was. The author of the article did not provide a similar explanation, or even an abbreviated one, in order to justify the claims that the bug does not have security implications. In fact, the author didn't even have to provide such an explanation, but he could have at least acknowledged the possibility, which could then be argued to be dismissable based on his (extensive) experience.
I'm not saying that there are security implications or that the burden of proving that there aren't any is on the author of the article.
I'm only agreeing with the parent poster that you cannot easily or simply claim that the bug is not a security vulnerability, without giving some consideration to the possible impact of UB, or at least acknowledging the possibility.
Not acknowledging this possible impact can in fact reinforce misconceptions about the security implications of bugs in C code that lead to UB.
lifthrasiir|2 years ago
Curl already runs ubsan and fuzzer in its testing process [1]. They know what they are doing (you may disagree on the first principles of writing curl in C/C++, but it's not like that curl is not sufficiently aware of UBs), so you should have asked instead why this particular bug was not caught already.
I believe the answer is that this bug occurred in the curl's CLI interface (src/tool_*.c), not in libcurl. The current fuzzer only runs against libcurl so this bug could have slipped in. But then it is even more clear that this bug can't be that serious, because libcurl does all the heavy lifting! Integer overflows themselves are not significant, they have to be paired with other mechanisms to be actually vulnerable (and Rust also agrees, whether overflow checks are enabled or not does not affect the memory safety).
Unless reporters have figured out an ingenious way to exploit a single integer overflow, the OP's reaction is completely justified.
[1] https://daniel.haxx.se/blog/2021/12/13/keeping-curl-safe/
wizeman|2 years ago
No, that is not true. That's a misconception. Your sentence is true for unsigned integer overflows, but not for signed integer overflows, which are undefined behavior in C.
Compilers assume that signed overflows are not possible. They exploit that assumption to perform many significant and important code optimizations, which greatly improve performance. Those optimizations can cause the program behavior to change dramatically in the case that a signed overflow would happen.
In some cases, the compiled code behaves in ways that have nothing to do with how the program was written.
In extreme (but real, documented) cases it can have effects like:
1. Security vulnerabilities being introduced in the compiled code, even though the source code appears to be perfectly safe.
2. Code that isn't called anywhere to be called.
3. The program crashing even though the code looks completely safe, and in fact would have been perfectly safe if the optimizations hadn't been performed.
4. And many more.
These are not theoretical or hypothetical effects. They are real effects that have been demonstrated in real code compiled with widely used compilers (GCC and clang), in some cases in high-profile projects like the Linux kernel, glibc and OpenSSL.
> and Rust also agrees, whether overflow checks are enabled or not does not affect the memory safety
That's because in Rust, signed integer overflows are well-defined.
In C, they are not well-defined. They are undefined behavior.
> Unless reporters have figured out an ingenious way to exploit a single integer overflow, the OP's reaction is completely justified.
I agree! It is completely justified.
However, the OP's assertions that the code is perfectly safe because it looks like a harmless integer overflow and that anyone can take a look at the code and figure out that it's not a vulnerability, are not justified.
It would be OK to say that the bug is unlikely to be a vulnerability. But not that it is not a vulnerability, at least, not without a much deeper look into why it is not (which would likely include looking at the disassembled code under multiple platforms, multiple compilers and multiple compiler versions).
To be clear, I believe that the burden of proving whether the bug is (or is not) a vulnerability is not on the OP. I would say it would be on those who have assigned a high severity to the CVE.
But that still doesn't make the assertions written by the OP correct.