4 ms·

It's a bit debatable whether this is a security bug vs a "you're holding it wrong" kind of bug, but fixing the bounds check is a good defensive practice and I'm

by moyix 3y ago

It's a bit debatable whether this is a security bug vs a "you're holding it wrong" kind of bug, but fixing the bounds check is a good defensive practice and I'm glad they implemented it. Somewhat tangentially, Microsoft at one point got in a bit of trouble for trying to use a randomized comparison function to implement shuffling on their "Browser choice" page: https://www.robweir.com/blog/2010/02/microsoft-random-browser-ballot.html https://www.robweir.com/blog/2010/02/microsoft-random-browse...

dralley 3y ago
Rust generally does consider "you're holding it wrong" type bugs to be safety issues. See for example [0], where the Rust developers filed a CVE for (and fixed) an issue in the standard library which the C / C++ communities mostly considered a holding-it-wrong issue. [0] https://www.reddit.com/r/cpp/comments/151cnlc/a_safety_culture_and_c_we_need_to_talk_about/ https://www.reddit.com/r/cpp/comments/151cnlc/a_safety_cultu...
- bregma 3y ago
  A you're-holding-it-wrong that still got fixed in all the major C++ standard library implementations.
  - pests 3y ago
    Fixed?
    - bregma 3y ago
      Code was added to libc++ and libstdc++ to avoid the TOCTOU exploit documented in the CVE. "Fixed" in this case means the source code for the library was modified to prevent the documented exploit, and test cases were added to their test suites to exercise the code. Not sure about the Microsoft C++ standard library since the Microsoft Windows system API does not provide the same softlink functionalty.
- jacquesm 3y ago
  But: they are holding it wrong and it got fixed regardless because if you can fix such stuff you should. For Rust such a CVE is nearly free given the installed base of Rust and the fact that most of that code is fresh and in maintenance. But that definitely isn't true for all of the instances of glibc that are currently deployed.
  - megous 3y ago
    Well, a fix in glibc fixed it across the whole system and for all already compiled programs. Can you say the same for Rust?
    - nequo 3y ago
      Dynamic linking is something that I would personally like to see in Rustland too. But parent's point is that some issues that count as a CVE for Rust do not count as such for C/C++ because the boundaries of undefined behavior are drawn elsewhere. This means that there are many fixes that are simply never made for C/C++, even though dynamic linking would make such a counterfactual fix easier to deploy.
      - megous 3y ago
        Parent point was that such CVE could be fixed in Rust super cheaply (free) because code is maintained. If I have to reinstall the whole system to fix CVE in Rust stdlib in a brave new world where everything is written in Rust, then it's certainly not free fix.
        nequo 3y ago
        I actually think you're right. Apologies. I also don't see deploying a glibc fix as particularly costly.
        jacquesm 3y ago
        But we don't live in that brave new world and we're talking about now, not some hypothetical.
- cozzyd 3y ago
  Is calling free with the wrong address a safety issue or "you're holding it wrong."
  - dureuill 3y ago
    It is both. "Memory safe" means that if the user is "holding it wrong", it won't cause a memory error. In safe Rust, it is impossible to call free with the wrong address (simply by virtue of the `free` equivalent being unsafe, and with the pervasive use of smart pointers to make it practical to stay in safe land). Similarly even if you "hold it wrong" and use a wrong comparison function in Rust's sort, you might get nonsensical results, but no memory errors. That's precisely what safety means in this context
- tialaramex 3y ago
  In particular, most traits you've seen for Rust, such as PartialEq, or here Ord (a claim that our type is totally ordered) are safe traits, this means you don't need to utter the unsafe keyword to implement these traits, and so even though Rust clearly documents the traits as having specific requirements in fact it promises it won't actually cause UB if you ignore them, and the users of that trait must behave accordingly. My misfortunate crate https://crates.io/crates/misfortunate https://crates.io/crates/misfortunate is a library of types that just deliberately implement various traits "wrongly" in order that you can play with the consequences. For example Clone promises to uh, clone things, but Rust can't know if this Doodad is really a "clone" of that Doodad, only that the result was really the correct type, so if your type only implements Default my Multiplicity type wrapper will cheerfully claim it can be cloned anyway, cloning the wrapper doesn't get you a "real" clone but only your Default - the types look correct but what's inside is not a "clone" in any reasonable sense. Rust does have what it calls unsafe traits, traits which you cannot implement without writing the "unsafe" keyword. They are solemn promises that you did what the documentation told you to, if you messed up in writing the unsafe trait then the resulting software may have Undefined Behaviour. For example, my type wrapper Comte (named after the guy who invented the trick with a hat you've seen magicians do) claims to be an ExactSizeIterator. If it has one rabbit inside it, duh, exact size. Nope, it's a trick, we can just tap the Comte with our wand and produce an unlimited number of rabbits despite claiming to be an ExactSizeIterator, that's a bad idea but it's not Undefined Behaviour. On the other hand Rust has the unsafe TrustedLen, which also says we can trust this iterator knows exactly how big it is, but because it's unsafe that's a solemn promise, if we took the Comte code but claimed to implement TrustedLen we're just despicable liars.
BoppreH 3y ago
I'm surprised that random comparison functions were not mentioned in the article. An awful idea, sure, but oh so tempting. Maybe everyone who tried it quickly saw the crashes...
tetha 3y ago
Mh, my main concern is: Both the weirdest absurd and non-exploitable things as well as the most trivially exploitable thing are just being run through all kinds of news channels as security vulnerabilities and out-of-bound writes and such. Like, "complete bypass of redirect URI validation in keycloak OIDC" vs "memory corruption for qsort in glibc (if the application passes a bad comparision function (and you can trigger it))". I'm finding myself very fatigued by this about vulnerabilities and find myself just giving up about trying to keep track. Also because there is no good curated source it feels. I can run after 3 nonsense ones a week and the one that can rip my systems apart drops on Christmas eve anyway.
- moyix 3y ago
  In theory this is what CVSS scores were supposed to fix, but a lot of the "does this vulnerability really matter" evaluation ultimately depends on the details of your environment and application :(
  - tetha 3y ago
    I have also found CVSS scores to over- or underestimate the impact of vulnerabilities so much.. I might as well flip a coin. And while I probably should be informed more, I really consider that a problem in the output of the security community. For example, the keycloak OIDC vulnerability would've allowed me to compromise any account of our internal authentication with ~1 bad click from a user, possibly fewer. Working that out to a point it was terrifying to all internal users took like 30 minutes at most and most time was wasted on realizing their version constraints for the exploit are wrong. That's a 7.8. But then you have "Oh curl with high parameters can hang" and "Oh if you reload postgres many times a second it stops working" coming in at 9+. Like sure, a user with elevated privileges on a critical server sending signals to postgres causing a postgres DoS is my biggest issue at that point. So I either have to evaluate everything for myself, or .. no idea.
    - AnthonyMouse 3y ago
      What I'd like to see is a different scoring system that measures how much attention I have to pay to something. Score 1: This is an implementation bug in the library. Update the library and you're done. It doesn't matter that this gives remote root on a machine with no services running, all you have to worry about is to install the patch and it's 100% fixed. Score 2: This is a vulnerability discovered in multiple implementations. Some of them have fixed it and some of them haven't. Here is the list of known-good implementations, you need to check that the one you're using is on the list and if it isn't then switch to one of the ones that is. Score 3: This is a flaw which some platforms don't currently provide a means to implement securely for any implementation. You need to pay attention to this if you use those platforms and possibly redesign your applications to do something else there. Score 4: This is a design flaw in the API. We can't fix it without breaking compatibility so here's a new API and now you definitely have to update your applications to use it and the existing ones will continue to be vulnerable. Compilers should emit a deprecation warning for anything still using the old API. Score 5: Spectre. We kind of mitigated it some and you need some patches but now you have to review all existing code, probably won't catch it all anyway, people will continue finding new variants for years and we're all totally screwed.

by moyix 3y ago

dralley 3y ago

Rust generally does consider "you're holding it wrong" type bugs to be safety issues. See for example [0], where the Rust developers filed a CVE for (and fixed) an issue in the standard library which the C / C++ communities mostly considered a holding-it-wrong issue. [0] https://www.reddit.com/r/cpp/comments/151cnlc/a_safety_culture_and_c_we_need_to_talk_about/ https://www.reddit.com/r/cpp/comments/151cnlc/a_safety_cultu...

bregma 3y ago
A you're-holding-it-wrong that still got fixed in all the major C++ standard library implementations.
- pests 3y ago
  Fixed?
  - bregma 3y ago
    Code was added to libc++ and libstdc++ to avoid the TOCTOU exploit documented in the CVE. "Fixed" in this case means the source code for the library was modified to prevent the documented exploit, and test cases were added to their test suites to exercise the code. Not sure about the Microsoft C++ standard library since the Microsoft Windows system API does not provide the same softlink functionalty.
jacquesm 3y ago
But: they are holding it wrong and it got fixed regardless because if you can fix such stuff you should. For Rust such a CVE is nearly free given the installed base of Rust and the fact that most of that code is fresh and in maintenance. But that definitely isn't true for all of the instances of glibc that are currently deployed.
- megous 3y ago
  Well, a fix in glibc fixed it across the whole system and for all already compiled programs. Can you say the same for Rust?
  - nequo 3y ago
    Dynamic linking is something that I would personally like to see in Rustland too. But parent's point is that some issues that count as a CVE for Rust do not count as such for C/C++ because the boundaries of undefined behavior are drawn elsewhere. This means that there are many fixes that are simply never made for C/C++, even though dynamic linking would make such a counterfactual fix easier to deploy.
    - megous 3y ago
      Parent point was that such CVE could be fixed in Rust super cheaply (free) because code is maintained. If I have to reinstall the whole system to fix CVE in Rust stdlib in a brave new world where everything is written in Rust, then it's certainly not free fix.
      - nequo 3y ago
        I actually think you're right. Apologies. I also don't see deploying a glibc fix as particularly costly.
      - jacquesm 3y ago
        But we don't live in that brave new world and we're talking about now, not some hypothetical.
cozzyd 3y ago
Is calling free with the wrong address a safety issue or "you're holding it wrong."
- dureuill 3y ago
  It is both. "Memory safe" means that if the user is "holding it wrong", it won't cause a memory error. In safe Rust, it is impossible to call free with the wrong address (simply by virtue of the `free` equivalent being unsafe, and with the pervasive use of smart pointers to make it practical to stay in safe land). Similarly even if you "hold it wrong" and use a wrong comparison function in Rust's sort, you might get nonsensical results, but no memory errors. That's precisely what safety means in this context
tialaramex 3y ago
In particular, most traits you've seen for Rust, such as PartialEq, or here Ord (a claim that our type is totally ordered) are safe traits, this means you don't need to utter the unsafe keyword to implement these traits, and so even though Rust clearly documents the traits as having specific requirements in fact it promises it won't actually cause UB if you ignore them, and the users of that trait must behave accordingly. My misfortunate crate https://crates.io/crates/misfortunate https://crates.io/crates/misfortunate is a library of types that just deliberately implement various traits "wrongly" in order that you can play with the consequences. For example Clone promises to uh, clone things, but Rust can't know if this Doodad is really a "clone" of that Doodad, only that the result was really the correct type, so if your type only implements Default my Multiplicity type wrapper will cheerfully claim it can be cloned anyway, cloning the wrapper doesn't get you a "real" clone but only your Default - the types look correct but what's inside is not a "clone" in any reasonable sense. Rust does have what it calls unsafe traits, traits which you cannot implement without writing the "unsafe" keyword. They are solemn promises that you did what the documentation told you to, if you messed up in writing the unsafe trait then the resulting software may have Undefined Behaviour. For example, my type wrapper Comte (named after the guy who invented the trick with a hat you've seen magicians do) claims to be an ExactSizeIterator. If it has one rabbit inside it, duh, exact size. Nope, it's a trick, we can just tap the Comte with our wand and produce an unlimited number of rabbits despite claiming to be an ExactSizeIterator, that's a bad idea but it's not Undefined Behaviour. On the other hand Rust has the unsafe TrustedLen, which also says we can trust this iterator knows exactly how big it is, but because it's unsafe that's a solemn promise, if we took the Comte code but claimed to implement TrustedLen we're just despicable liars.

BoppreH 3y ago

I'm surprised that random comparison functions were not mentioned in the article. An awful idea, sure, but oh so tempting. Maybe everyone who tried it quickly saw the crashes...

tetha 3y ago

Mh, my main concern is: Both the weirdest absurd and non-exploitable things as well as the most trivially exploitable thing are just being run through all kinds of news channels as security vulnerabilities and out-of-bound writes and such. Like, "complete bypass of redirect URI validation in keycloak OIDC" vs "memory corruption for qsort in glibc (if the application passes a bad comparision function (and you can trigger it))". I'm finding myself very fatigued by this about vulnerabilities and find myself just giving up about trying to keep track. Also because there is no good curated source it feels. I can run after 3 nonsense ones a week and the one that can rip my systems apart drops on Christmas eve anyway.

moyix 3y ago
In theory this is what CVSS scores were supposed to fix, but a lot of the "does this vulnerability really matter" evaluation ultimately depends on the details of your environment and application :(
- tetha 3y ago
  I have also found CVSS scores to over- or underestimate the impact of vulnerabilities so much.. I might as well flip a coin. And while I probably should be informed more, I really consider that a problem in the output of the security community. For example, the keycloak OIDC vulnerability would've allowed me to compromise any account of our internal authentication with ~1 bad click from a user, possibly fewer. Working that out to a point it was terrifying to all internal users took like 30 minutes at most and most time was wasted on realizing their version constraints for the exploit are wrong. That's a 7.8. But then you have "Oh curl with high parameters can hang" and "Oh if you reload postgres many times a second it stops working" coming in at 9+. Like sure, a user with elevated privileges on a critical server sending signals to postgres causing a postgres DoS is my biggest issue at that point. So I either have to evaluate everything for myself, or .. no idea.
  - AnthonyMouse 3y ago
    What I'd like to see is a different scoring system that measures how much attention I have to pay to something. Score 1: This is an implementation bug in the library. Update the library and you're done. It doesn't matter that this gives remote root on a machine with no services running, all you have to worry about is to install the patch and it's 100% fixed. Score 2: This is a vulnerability discovered in multiple implementations. Some of them have fixed it and some of them haven't. Here is the list of known-good implementations, you need to check that the one you're using is on the list and if it isn't then switch to one of the ones that is. Score 3: This is a flaw which some platforms don't currently provide a means to implement securely for any implementation. You need to pay attention to this if you use those platforms and possibly redesign your applications to do something else there. Score 4: This is a design flaw in the API. We can't fix it without breaking compatibility so here's a new API and now you definitely have to update your applications to use it and the existing ones will continue to be vulnerable. Compilers should emit a deprecation warning for anything still using the old API. Score 5: Spectre. We kind of mitigated it some and you need some patches but now you have to review all existing code, probably won't catch it all anyway, people will continue finding new variants for years and we're all totally screwed.