3 ms·

You are technically correct, but signed integer overflows are so pervasive in C that compilers only exploit them when ignoring them would be truly beneficial.

You are technically correct, but signed integer overflows are so pervasive in C that compilers only exploit them when ignoring them would be truly beneficial. In the case of LLVM for example signed integer overflows will merely produce a poison value, and won't produce the actual UB if it is either optimized out or only used by instructions that can't produce UBs by themselves [1]. Compilers also tend to use the UB as an excuse to optimize things, not to break things. We only care about the UB because a faulty optimization still can cause breakages. In this particular case the poison value will eventually reach the following condition [2]: /* if retry-max-time is non-zero, make sure we haven't exceeded the time */ if(per->retry_numretries && (!config->retry_maxtime || (tvdiff(tvnow(), per->retrystart) < config->retry_maxtime*1000L)) ) { So let's assume that `config->retry_maxtime * 1000L` was indeed always a poison value---what can reasonable optimizing compilers do? At worst they will probably evaluate the entire condition as false, because it would help the dead code elimination. Compilers can put a nasal demon in that case, but reasonable ones wouldn't. And since `config->retry_maxtime * 1000L` is not guaranteed to be a poison value all the time, this optimization is not even actually possible. The practical effect of this security bug is quite limited. [1] https://llvm.org/docs/LangRef.html#poison-values https://llvm.org/docs/LangRef.html#poison-values [2] https://github.com/curl/curl/blob/c2212c05aa99bb31e4b99b0b66fc1747b7d01be6/src/tool_operate.c#L473-L478 https://github.com/curl/curl/blob/c2212c05aa99bb31e4b99b0b66... (similar for `config->retry_delay`, omitted for brevity here)

wizeman 3y ago
> The practical effect of this security bug is quite limited. 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 3y ago
  > 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. 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/ https://daniel.haxx.se/blog/2021/12/13/keeping-curl-safe/
  - wizeman 3y ago
    > Integer overflows themselves are not significant, they have to be paired with other mechanisms to be actually vulnerable 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.
rocqua 3y ago
In order for the execution to exhibit undefined behavior, on windows it would require 25 days as a retry period, whilst Linux would require much more. In practice the likelihood of someone passing that value is rather low.

wizeman 3y ago

> The practical effect of this security bug is quite limited. 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 3y ago
> 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. 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/ https://daniel.haxx.se/blog/2021/12/13/keeping-curl-safe/
- wizeman 3y ago
  > Integer overflows themselves are not significant, they have to be paired with other mechanisms to be actually vulnerable 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.

rocqua 3y ago

In order for the execution to exhibit undefined behavior, on windows it would require 25 days as a retry period, whilst Linux would require much more. In practice the likelihood of someone passing that value is rather low.