Undefined Behaviour as usual [LWN.net]

Undefined Behaviour as usual

Posted Feb 23, 2024 18:12 UTC (Fri) by Sesse (subscriber, #53779) [Link] (24 responses)

Can you point to any (remotely real) system where this actually causes a security issue?

Undefined Behaviour as usual

Posted Feb 23, 2024 18:15 UTC (Fri) by mb (subscriber, #50428) [Link] (6 responses)

No. Because I have not tried to find one.

But neither can curl ensure there is no such system anywhere.

Undefined Behaviour as usual

Posted Feb 23, 2024 18:25 UTC (Fri) by Sesse (subscriber, #53779) [Link] (5 responses)

Well, normally, the burden of proof is on the accuser? I mean, I'm pretty sure curl will hit UB in various ways if you have e.g. int at 18 bits (presumably with 9-bit bytes), which is also not impossible at all as per the C standard. Would you file curl CVEs for every code that would behave incorrectly on such a system? After all, curl certainly cannot rule out that such a system exists somewhere. Since you seem to assume -fwrapv would nullify this bug (even though it does absolutely nothing to change the C standard), what about compilers where such an option does not exist?

Sometimes, you can do bad things (have bugs) without them turning into a security vulnerability in practice.

Undefined Behaviour as usual

Posted Feb 23, 2024 18:35 UTC (Fri) by mb (subscriber, #50428) [Link] (2 responses)

>Well, normally, the burden of proof is on the accuser?

No. That is not how security works.

> if you have e.g. int at 18 bits (presumably with 9-bit bytes),

What-about-ism.
Has nothing to do with signed-overflow being UB.

>Since you seem to assume -fwrapv would nullify this bug

No. I didn't say that. I was talking about UB. Not whether the bug would still be a bug or not.

Undefined Behaviour as usual

Posted Feb 27, 2024 11:02 UTC (Tue) by JustABug (guest, #169930) [Link]

A bug can just be a bug.
It doesn't need to have a CVE entry to be remediated quickly.

Does this UB allow for priviledge escalation? Data expositon? What's the attack vector? User intentionally entering a stupid value?

If the user can run curl they can run rm -rf

What's the output? Program crash? Exploitable unintended behaviour? What's an abuse scenario?

The researcher filing the CVE needs to demonstrate their CVE isn't a nothing burger.

The only advantage I can think of for filing a CVE for every UB is ensuring the fix is backported. Using BS CVEs as a tool to get things backported is an abuse of the system to address the problem of selective backporting.

Undefined Behaviour as usual

Posted Mar 22, 2024 18:58 UTC (Fri) by DanilaBerezin (guest, #168271) [Link]

> No. That is not how security works.

That is how security works though? Any line of code has the potential to be exploited. If the mere possibility of an exploit is the bar we set to file a CVE, then I can mark every line of code in every project as a CVE. Obviously, that would be very silly.

Undefined Behaviour as usual

Posted Feb 23, 2024 18:41 UTC (Fri) by wtarreau (subscriber, #51152) [Link] (1 responses)

What's fun is that often the people who go through extreme mind-stretching to imagine a possible vulnerability in a random bug are the same who complain about the excess of CVEs reported in new stable kernels, which just apply a softened version of their logic :-)

Undefined Behaviour as usual

Posted Feb 24, 2024 0:19 UTC (Sat) by mb (subscriber, #50428) [Link]

Do you have an actual example of such a person?
I could not imagine a single one.

Undefined Behaviour as usual

Posted Feb 23, 2024 21:57 UTC (Fri) by bagder (guest, #38414) [Link] (16 responses)

No they can't. This, as many other of these UB problems are mostly *theoretical* problems in imaginary systems that *might* "do anything". Blindly considering all UBs to be security problems is not helping the world.

I consider it a fairly important property that a security problem should be possible to actually happen on existing hardware where the code was built with existing compilers. If not, it actually is not a security problem and we don't do any service to the world by treating as such.

Undefined Behaviour as usual

Posted Feb 23, 2024 22:08 UTC (Fri) by mb (subscriber, #50428) [Link] (15 responses)

>we don't do any service to the world by treating as such.

That means you'll have to constantly re-assess old UB bugs with every new compiler and system release.
That's an almost infinite amount of work.
Treating UB bugs as security bugs and fixing them right away is easy compared to that.

Undefined Behaviour as usual

Posted Feb 24, 2024 1:03 UTC (Sat) by jwarnica (subscriber, #27492) [Link] (13 responses)

We already have to reassess all behavior, even defined behavior, of new compiler and system releases, as the compiler and system itself may be buggy. Or less buggy, but previously relied upon buggy behavior changed in usually good, but one particular situation, bad ways.

Undefined Behaviour as usual

Posted Feb 24, 2024 11:53 UTC (Sat) by mb (subscriber, #50428) [Link] (12 responses)

No. Finding newly introduced misbehavior is a completely different class of task than re-assessing known UB bugs over and over again for actual misbehavior.
Because re-assessing known UB bugs is completely unnecessary, if they are understood as ticking time bombs that can trigger undefined behavior (including security issues) at any point in time if the ever so slightest bit of environment changes.

UB bugs must be documented as such and must get fixed right away.
Rejecting a CVE is the opposite of that. It is not a responsible reaction to an UB bug. It is hiding your head in the sand.
Documenting and fixing the UB *solves* the problem once and for all. No re-assessment needed.

Undefined Behaviour as usual

Posted Feb 24, 2024 12:07 UTC (Sat) by jwarnica (subscriber, #27492) [Link] (11 responses)

UB is UB in the spec. It is defined in the implementation.

DB is DB in the sepc. It is further defined (possibly incorrectly) in the implementation.

System v+1 might change both of these scenarios.

The only difference if v+1 changes your application is how righteous your bug report to the system is.

Undefined Behaviour as usual

Posted Feb 24, 2024 12:33 UTC (Sat) by mb (subscriber, #50428) [Link] (1 responses)

>System v+1 might change both of these scenarios.

Correct.
Upon system change: Having to check two things or having to check only one out of the two. What is better?

Also: The defined-behavior check is being mostly done by the release tests of the system components.

> UB is UB in the spec. It is defined in the implementation.

That is not true.
Data race UB is not defined in the implementation anywhere. It cannot be defined. It is non-deterministic. That's just one example.

You are talking about Implementation-Defined and just assume that UB is the same thing.
It is not.

Undefined Behaviour as usual

Posted Mar 1, 2024 15:48 UTC (Fri) by mathstuf (subscriber, #69389) [Link]

> Data race UB is not defined in the implementation anywhere. It cannot be defined. It is non-deterministic. That's just one example.

The concrete behavior is non-deterministic, sure. However, the language spec can say things like "must treat the value as valid" even if its value is not what is expected (or even naïvely possible given the code). Instead, we have "if a data race happens, the bits in the memory must not be interpreted". The JVM has the former (you get *some* behavior of the code you wrote even if it's not what you expect). C has the latter (if the compiler notices a data race, it can instead assume it never happens and optimize accordingly; this may be "no behavior" after optimization passes complete). Note that C compilers don't stick in "if we notice UB at runtime, do_bad_thing()" code (unless requested via UBSAN and friends). Instead, their noticing of UB ends up affecting how optimizations treat the code which *then* transform it into unexpected logic flows based on the raw syntax of the code available.

In this sense, I think that overflowing on a command line argument parse is unlikely to have any real effect as the optimizer is just going to assume it stays in range and "means something". However, instead we have "the timeout parameter takes an integer, multiplication makes an integer…maybe it overflows, but who cares as 'it doesn't happen'". The code is the same whether overflow happens or not…it's just that the value is not interpreted as 1000*input but as 1000*input % UINT_MAX (or whatever type it is). Given the stratospheric values needed to overflow, I dare say that anyone flirting with these values already had a DoS on their plate in case the intended timeout actually ended up having to expire. It's only a real problem for those running with UBSAN on…but they're asking for DoS crashes by doing so anyways.

IMO, should UB be patched and guarded against? Yes. Does *this* instance warrant a CVE? No. `curl | sh` with bogus timeout parameters is more easily handled with replacing a URL with a download from malicious.site instead.

Undefined Behaviour as usual

Posted Feb 27, 2024 11:21 UTC (Tue) by danielthompson (subscriber, #97243) [Link] (8 responses)

> UB is UB in the spec. It is defined in the implementation.

I think this confuses undefined behavior (e.g. reading past the end of an array) with implementation defined behavior (e.g. signess of char, size of int, etc). In some cases (such as reading past the end of an array whose length is not known at compile time) then compiler implementation cannot define what happens!

Signed integer overflow is interesting because it is an undefined behaviour (according to the C spec) and many implementations really do make it's behavior undefined. For example the behavior on overflow can and does change based on the optimization level. Nevertheless it is possible for an implementation to fully define what happens on overflow if the application is willing to accept missed optimization opportunities (hence -fwrapv).

It is also interesting because one of the common undefined behaviors linked to signed overflow is the removal of security bounds checks that (incorrectly) assumed integer overflow would wrap... and this certainly did lead to vulnerabilities.

However, despite the above, I'm unconvinced by the what-about-ism in the "every overflow is a ticking bomb" argument": the set of plausible optimizations on a time conversion during command line processing is relatively small.

Undefined Behaviour as usual

Posted Feb 29, 2024 8:35 UTC (Thu) by anton (subscriber, #25547) [Link]

There is no way to "really make it's behaviour undefined", except by compiling to code where the architecture makes the behaviour undefined. Sane computer architects don't, and even insane architects don't do it for anything that a compiler is likely to output for signed addition. When reading past the end of an array, the code produced by the compiler usually just reads past the end of the array; it takes special effort by the compiler writer to do something different; but yes, compiler writers have taken this special effort, with the result being that there are cases of "optimizing" a loop with a read past the end of an array into an infinite loop.

The removal of bounds checks is another such "optimization"; however, it is often based on the assumption that pointer arithmetic (not signed integer arithmetic) does not wrap around. Therefore, to avoid this kind of miscompilation, -fwrapv is insufficient. The better option is -fno-strict-overflow (which combines -fwrapv and -fwrapv-pointer).

Undefined Behaviour as usual

Posted Feb 29, 2024 11:47 UTC (Thu) by SLi (subscriber, #53131) [Link] (6 responses)

I think what the parent may have wished to say is that undefined behavior, in some sense, becomes defined behavior once you are looking at the binary, possibly sometimes together with some environmental details.

While I'm all for making code robust and even moving to safer languages, I think most people are more interested in vulnerabilities in actual binaries running on actual computers than in ones where a theoretical, possibly evil compiler could read the spec and perform a theoretically valid compilation where the output does something bad.

Undefined Behaviour as usual

Posted Feb 29, 2024 14:31 UTC (Thu) by pizza (subscriber, #46) [Link] (5 responses)

> I think what the parent may have wished to say is that undefined behavior, in some sense, becomes defined behavior once you are looking at the binary, possibly sometimes together with some environmental details.

Yeah, that's my take on this too.

It's "undefined" in the spec, but the actual compiled binary (+runtime environment) exhibits highly consistent (albeit unexpected/unintended) behavior. After all, script kiddies couldn't exploit those bugs into a privilege escalation without the binary behaving in this implementation-specific manner.

Undefined Behaviour as usual

Posted Feb 29, 2024 19:10 UTC (Thu) by farnz (subscriber, #17727) [Link] (4 responses)

Am I right in thinking that you'd agree that it's the output binary that's usually deterministic in its environment (modulo things with clearly-defined non-determinism such as the RDRAND instruction, or runtime data races between multiple threads), and not the combination of compiler (including flags) and source code?

In other words, while this would be a nasty surprise, you wouldn't be surprised if recompiling the same UB-containing source resulted in a different binary, but you would be surprised if using the same binary in the same environment with the same inputs had a different output, unless it was doing something that is clearly specified to have non-deterministic behaviour by the hardware (like a data race, or RDRAND).

Undefined Behaviour as usual

Posted Mar 1, 2024 1:37 UTC (Fri) by pizza (subscriber, #46) [Link] (3 responses)

> Am I right in thinking that you'd agree that it's the output binary that's usually deterministic in its environment (modulo things with clearly-defined non-determinism such as the RDRAND instruction, or runtime data races between multiple threads), and not the combination of compiler (including flags) and source code?

Yes, I'd agree with this, and I don't think that it's a terribly controversial opinion.

> In other words, while this would be a nasty surprise, you wouldn't be surprised if recompiling the same UB-containing source resulted in a different binary

If you're recompiling with the same toolchain+options I'd expect the resulting binaries to behave damn near identically from one build to the next [1]. (Indeed, as software supply-chain attacks become more widespread, 100% binary reproducibility is a goal many folks are working towards)

> but you would be surprised if using the same binary in the same environment with the same inputs had a different output, unless it was doing something that is clearly specified to have non-deterministic behaviour by the hardware (like a data race, or RDRAND).

Yep. After all, the script kiddies wouldn't be able to do their thing unless a given binary on a given platform demonstrated pretty consistent behavior.

[1] The main difference would the linker possibly putting things in different places (especially if multiple build threads are involved) but that doesn't change the fundamental attack vector -- eg a buffer overflow that smashes your stack on one build (and/or platform) will still smash your stack on another, but since the binary layout is different, you'll likely need to adjust your attack payload to achieve the results you want. Similarly, data-leakage-type attacks (eg Heartbleed) usually rely on being able to repeat the attack with impunity until something "interesting" is eventually found.

Undefined Behaviour as usual

Posted Mar 1, 2024 10:04 UTC (Fri) by farnz (subscriber, #17727) [Link] (2 responses)

If you're recompiling with the same toolchain+options I'd expect the resulting binaries to behave damn near identically from one build to the next [1]. (Indeed, as software supply-chain attacks become more widespread, 100% binary reproducibility is a goal many folks are working towards)

This is a potentially dangerous expectation in the presence of UB in the source code; there are optimizations that work by searching for a local maximum, and where for fully-defined code (even where it's unspecified behaviour, where there are multiple permissible outcomes), we know that there is only one maximum they can find. We use non-determinism in that search to speed it up, and for UB we run into the problem that there can be multiple maxima, all of which are locally the best option.

Because the search is non-deterministic, exactly which maximum we end up in for some UB cases is also non-deterministic. This does mean that 100% binary reproducibility has the nice side-effect of wanting to reduce UB - by removing UB, you make the search type optimizations find the one and only one optimal stable state every time, instead of choosing a different one each time).

And I'd agree that it's not terribly controversial to believe that a binary running in user mode has no UB - there's still non-deterministic behaviour (like the result of a data race between threads, or the output of RDRAND), and if your binary's behaviour is defined by something non-deterministic, it could end up in what the standards call unspecified behaviour. This is not universally true once you're in supervisor mode (since you can do things on some platforms like change power rails to be out-of-spec, which results in CPUs having UB since the logic no longer behaves digitally, and thus it's possible for software UB to turn into binary defined behaviour of changing platform state such that the platform's behaviour is now unpredictable).

Undefined Behaviour as usual

Posted Mar 1, 2024 14:48 UTC (Fri) by pizza (subscriber, #46) [Link] (1 responses)

> we know that there is only one maximum they can find. We use non-determinism in that search to speed it up, and for UB we run into the problem that there can be multiple maxima, all of which are locally the best option.

FWIW I've seen my fair share of shenanigans caused by nondeterminsitic compiler/linking behavior. To this day, there's one family of targets in the CI system that yields a final binary that varies by about 3KB from one build to the next depending on which nonidentical build host was used to cross-compile it. I suspect that is entirely due to the number of cores used in the highly parallel build; I've never seen any variance from back-to-back builds on the same machine (binaries are identical except for the intentionally-embedded buildinfo)

But I do understand what you're saying, and even agree -- but IME compilers are already very capable of loudly warning about the UB scenarios that can trigger what you described. Of course, folks are free to ignore/disable warnings, but I have no professional sympathy for them, or the consequences.

> This is not universally true once you're in supervisor mode (since you can do things on some platforms like change power rails to be out-of-spec [...]

I've spent most of my career working in bare-metal/supervisory land, and yeah, even a off-by-one *read* could have some nasty consequences depending on which bus address that happens to hit. OTOH, while the behavior of that off-by-one read is truly unknowable from C's perspective, if the developer "corrects" the bug by incrementing the array size by one (therefore making it too large for the hw resource) the program is now "correct" from C's perspective, but will trigger the same nasty consequences on the actual hardware.

Undefined Behaviour as usual

Posted Mar 1, 2024 16:34 UTC (Fri) by farnz (subscriber, #17727) [Link]

OTOH, while the behavior of that off-by-one read is truly unknowable from C's perspective, if the developer "corrects" the bug by incrementing the array size by one (therefore making it too large for the hw resource) the program is now "correct" from C's perspective, but will trigger the same nasty consequences on the actual hardware.

I spend a lot of time in the land of qualified compilers, where the compiler promises that as long as you stick to the subset of the language that's qualified, you can look only at the behaviours evident in the source code to determine what the binary will do. You're expected, if you're working in this land, to have proper code review and separate code audit processes so that a fix of the type you describe never makes it to production, since it's obvious from the source code that the program, while "correct" from C's perspective, is incorrect from a higher level perspective.

And a lot of the problems I see with the way UB is handled feel like people expect all compilers to behave like qualified compilers, not just on a subset of the language, but on everything, including UB.

Undefined Behaviour as usual

Posted Feb 26, 2024 13:16 UTC (Mon) by bagder (guest, #38414) [Link]

Bugs should be fixed yes.

An UB is a bug that *might* have a security impact. We do not make the world a better place by blindly assuming every UB is a security vulnerability. That's just like crying wolf. It is not helpful. Proper security assessment should still be applied.

Since they are bugs that should be fixed, we don't have to "come back" later to reassess their security impact. Unless we reintroduce the bugs of course.

Those are guidelines I adhere to in the projects I work in.

Undefined Behaviour as usual

Posted Feb 24, 2024 5:57 UTC (Sat) by jmspeex (subscriber, #51639) [Link] (3 responses)

> One cannot rule out security implications without knowing the whole system in that case.

Correct. But then that is also true of most bugs you find in software. Based on that alone almost all bugs should be filed at the highest severity, which isn't exactly helpful. Any UB should get fixed, but when UB gets discovered in a piece of code, someone has to make a reasonable effort to figure out how bad the impact is likely to be. Write out of bounds on the stack: very very bad. Integer wrap-around in a loop counter: could be bad unless careful analysis shows it's unlikely to be. Integer overflow in the value that only gets printed to a log: much less likely to be exploited unless proven otherwise.

Undefined Behaviour as usual

Posted Feb 24, 2024 6:35 UTC (Sat) by Otus (subscriber, #67685) [Link] (2 responses)

> Based on that alone almost all bugs should be filed at the highest severity, which isn't exactly helpful.

Having a CVE doesn't imply highest severity, even low severity vulnerabilities are meant to have one. Severity analysis is a separate matter.

Undefined Behaviour as usual

Posted Feb 24, 2024 6:39 UTC (Sat) by jmspeex (subscriber, #51639) [Link] (1 responses)

Well, if you look at the issue being discussed, it was rated as severity 9.8/10. If you're going to give that rating for any integer overflow because (technically it's UB), then you have no room left for the scary stuff.

Undefined Behaviour as usual

Posted Feb 24, 2024 11:30 UTC (Sat) by Otus (subscriber, #67685) [Link]

I can easily believe that the severity was wrong. But shouldn't that then be fixed?

I don't really know what the correct severity would've been here, but the severity part has always been black magic. (I don't think those are particularly useful in practice.)

My point is simply that CVE isn't supposed to be exclusively for highest impact issues, but any vulnerabilities.

Undefined Behaviour as usual

Posted Feb 29, 2024 8:49 UTC (Thu) by anton (subscriber, #25547) [Link]

The gcc option -fno-strict-overflow would imply -fwrapv (but I have not looked in the sources whether curl is compiled with that).

Yes, unless you are compiling a benchmark, compiling with -fno-strict-overflow is a good idea. This limits the kinds of shenenigans that the compilers do, a little. There are also a number of other such flags.

Actually, if we consider C "undefined behaviour" to be a security issue all by itself, then a C compiler that does not (by default) limit what it does is a security issue all by itself. Maybe someone (a Rust fan?) should file one CVE for each undefined behaviour (211 in C11 IIRC) for each C compiler, unless that compiler limits that behaviour for that case by default (offering flags like -fstrict-overflow for compiling benchmarks is allowed, of course).