4 ms·
I see the glibc security team fell back to a variant of the age old "oh, but it's undefined behavior, so if we burn down your computer that's okay too!" justifi
by sgift 3y ago
I see the glibc security team fell back to a variant of the age old "oh, but it's undefined behavior, so if we burn down your computer that's okay too!" justification why they shouldn't be assigned an CVE for this.
> This memory corruption in the GNU C Library through the qsort function is
invoked by an application passing a non-transitive comparison function, which
is undefined according to POSIX and ISO C standards. As a result, we are of
the opinion that the resulting CVE, if any, should be assigned to any such
calling applications and subsequently fixed by passing a valid comparison
function to qsort and not to glibc.
Disappointing. Not unexpected, but still disappointing. Oh well, at least they fixed it.
- CJefferson 3y agoGcc's std::sort has always had this "bug" for bad comparators, doing "x <= y" instead of "x < y" is enough to cause out of bounds read and writes. This isn't fixed because it's undefined behaviour.
- alphazard 3y agoTheir assessment seems fair to me. There are lots of reasons that software can be bad. "Easy to misuse" is one of them. Not the same thing as a vulnerability. I'm sure there are some Rust devs who would say the same thing about C. It's possible to write secure code in C, just as it's possible to write a sort with defined behavior using glibc.
- sgift 3y agoThe reason I'd say assigning a CVE is the right call is that the "easy to misuse" leads to a potential security problem here because of the way glibc handles the function that you give it. Case in point: You can make the same error in Java (I think it's even included in the docs for Comparators that you shouldn't do a-b, because of integer overflow/underflow), but it cannot lead to an out-of-bounds read/write. And that's because Java (~glibc) handles the comparison function that you give it differently.
- hedora 3y agoIt can't lead to an out of bounds write, but it could violate higher-level invariants in the Java program, and those could be exploitable. For instance, the bad comparator could be used in nested authentication checks: if check_A { drop privilege to FOO } else if check_B { drop privilege to BAR } else { /* leave root bit set, because "not (A or B)" means root */ } and it could lead to a privilege escalation in the face of malformed inputs. I've seen this sort of thing happen in the wild with Java.
- skitter 3y agoI don't think it's undefined behavior though. Looking at (the working draft of) the C ISO standard, I don't see a requirement that the comparison function must be transitive. The closes thing there is 7.22.5.2 §4: "If two elements compare as equal, their order in the resulting sorted array is unspecified" So I'd say blaming users for a bug in glibc isn't fair.
- int_19h 3y agoIn ISO C17, 7.2.5/4 has the relevant verbiage: "When the same objects (consisting of size bytes, irrespective of their current positions in the array) are passed more than once to the comparison function, the results shall be consistent with one another. That is, for qsort they shall define a total ordering on the array, and for bsearch the same object shall always compare the same way with the key."
- skitter 3y agoThanks! So it is/was indeed UB.
- voakbasda 3y agoIf the person using a foot gun shoots off their toe, do we blame the foot gun manufacturer? No, we blame the person that did not take proper training or exercise sufficient caution when using their tool.
- xboxnolifes 3y agoWe very frequently blame footgun manufacturers for manufacturing footguns.
- nequo 3y agoThis one is not on GNU libc then but on ISO C and POSIX because they manufactured this particular footgun.
- debugnik 3y agoBut being footguns is a feature of C/C++, not a defect: they trade safety for performance by design. (Edit: Wrong trade-off in my opinion, just to make it clear.) If you shop specifically for an unsafe tool, surprise, you get one. Should have picked better.
- dralley 3y ago1) The performance saved is negligible in many cases, for most footguns. Sometimes it's of no benefit, or slower than safe solutions to the same problem. 2) There's no reason to not just have the footgun be opt-in rather than it being the default approach, if performance does matter so much 3) Why is trading safety for performance the right decision in the first place? It's funny to compare the way software engineers talk about software with the way any other kind of engineer talks about their domain, or even how software engineers talk about other domains. How many threads have been had here over the past month about how unacceptable Boeing's "just turn off the de-icer" is to a hardware problem that could damage the engine, and other dodgy engineering decisions?
- debugnik 3y ago
- gnfargbl 3y agoThe glibc team could certainly have been a little less guarded, but I'm not sure they were wrong. If CVEs are to be something other than a stamp collection, then they need to facilitate remediation. To do that, CVEs need to identify vulnerabilities specifically and not just in general. For example, it wouldn't be useful to assign a CVE to the concept of SQLi, but it can be useful to assign CVEs to particular products exhibiting SQLi vulnerabilities. This fault isn't quite as general as that, but the argument for non-CVE-assignment goes along the same lines.
- jacquesm 3y agoCVE's aren't free. The aftermath of issuing a CVE for this bug would have massive consequences all over the industry, fixing the bug and leaving the past - where you probably would have become aware of this by now if it affected you and you probably should have a look at your code regardless to see if the situation can occur - means that on the next scheduled update of glibc everybody will have the fix. If you have a way of getting this to escalate to RCE or some other nastiness, especially if it can be done remotely in some application that is networked and that uses qsort (which should be most of them so if it is that easy then it should be quick to prove). Note that even something as mundane as the UTF-8 encoding scheme led to an exploit so it may well be possible. But I think classifying this as a security bug rather than just as a bug at this point is premature. Consider being on the receiving side of a mandatory (for instance: regulated industry) upgrade requirement of all of your systems because a CVE was issued would carry substantial costs even if the risk of those systems being exploited was very small. So the glibc team likely took such costs into account and absent an easy or obvious way to exploit this that seems for the moment to be the right call. But it could change.
- n_plus_1_acc 3y agoRequiring no CVE is obviously a proxy for no vulnerabilities, and it's as close as you can get. I think if you are in an industry that values this, the correct thing to do is check if you use qsort and check your comparision functions. Now. To encourage this, it would be good thing for glibc to issue a CVE.