6 ms·

A fun one. Buffer overflows tend to usually get associated with providing too much data; here's a nice case in which an overflow is triggered by providing too l

A fun one. Buffer overflows tend to usually get associated with providing too much data; here's a nice case in which an overflow is triggered by providing too little. Seems like the buffer for storing the password was changed to be dynamically allocated, but only in some parts of the code; other parts still treated it as something that is at least nine bytes long (including the null terminator). In practice, this means that if your password is only one char, then the actual buffer is two bytes long, and the seventh byte past the buffer is then zeroed/set to the null terminator. I wonder if and how this is exploitable.

dottedmag 4y ago
Probably only with severely niche libc. Is there any libc that allocates data with granularity smaller than 16 bytes?
- nn3 4y ago
  Very doubtful there is one. Seems like a total nothing burger
  - tinus_hn 4y ago
    Sudo is a security boundary, it has to be rock solid and an issue that doesn’t immediately look exploitable is still a big deal. Sudo runs under the control of the attacker, it’s playing with fire!
    - ilyt 4y ago
      Case in point: the whole speculative execution was only suspected to maybe be exploitable decades ago, and only now we have a bunch of PoCs
      - userbinator 4y ago
        ...which are still only useful under highly contrived conditions which require knowing so much about the target that it wouldn't be a practical concern.
st_goliath 4y ago
> the seventh byte past the buffer is then zeroed/set to the null terminator. I wonder if and how this is exploitable. Well, looking at the upstream code, the original value is saved away before zeroing it out and then unconditionally restored after running crypt. If sudo is single threaded and crypt() doesn't malloc() anything, I don't think that can be exploited. Worst case would be a segfault if the password was somehow close enough to a page boundary.
- geocar 4y ago
  > I don't think that can be exploited. glibc malloc() should be aligned to 2*sizeof(size_t), so strup("")[x] on 64-bit systems (with 16-byte alignment) can never crash or overlap another object where x<16 On 32-bit systems and with other mallocs you could potentially be reaching another page (like I think you are imagining) or trashing some bookkeeping bits which might crash free() but I cannot yet see how you would induce that, nor convince myself it cannot be done without spending more time thinking about it (something I'm reluctant to do with my afternoon)
userbinator 4y ago
Seems like the buffer for storing the password was changed to be dynamically allocated When I see things like this, the first question I have is why? A password isn't going to be long enough to require dynamic allocation, so just use a fixed-size buffer. 255 is already generous and a good round number. The best solutions are often also the simplest.
- AlexanderDhoore 4y ago
  This is why I love working in embedded. Our clients would never complain about these kinds of reasonable restrictions. There are tons of limits like this in our machines, which nobody notices but make my life much easier. I guess things are different in the Linux/PC software world.
  - tgv 4y ago
    It also might explain the S in IoT.
    - ilyt 4y ago
      I'd say not anymore, average microcontroller used in IoT got fat enough, nowadays even smaller chips come with AES acceleration too. I'm frankly surprised some proper standard didn't pop up already, I guess closing the users in your own ecosystem as fast as possible is priority in the industry. Good 80-90% of devices could be just "an MQTT connection + a bit of code to pair it up initially and feed server data" + a bunch of templates for how typical services should present themselves (so a light will just work with any compatible "control center"). Then just sell user subscription to your cloudy cloud IFTTT clone or a separate box to put in house that does the same job.
- marcosdumay 4y ago
  Because then you'll have another vulnerability created by too long passwords.
  - GTP 4y ago
_kbh_ 4y ago
> I wonder if and how this is exploitable. Would be at worst a memory read past the inputted password, which wouldn't be super useful outside of a leak for another vulnerability but even that seems unlikely.
- TonyTrapp 4y ago
  No, the code overwrote the 9th byte of the buffer to add a null terminator: https://github.com/sudo-project/sudo/commit/bd209b9f16fcd1270c13db27ae3329c677d48050 https://github.com/sudo-project/sudo/commit/bd209b9f16fcd127... So it's not just reading past the end of the buffer, but it's overwriting a single byte potentially belonging to another object. It may still cause a crash but it's relatively unlikely that it could cause something more severe.
  - Someone 4y ago
    I don’t understand how this can be correct: if (pw_len == DESLEN || HAS_AGEINFO(pw_epasswd, pw_len)) { strlcpy(des_pass, pass, sizeof(des_pass)); pass = des_pass; } Doesn’t that truncate your password if it happens to have DESLEN characters?
  - _kbh_ 4y ago
    > No, the code overwrote the 9th byte of the buffer to add a null terminator: https://github.com/sudo-project/sudo/commit/bd209b9f16fcd127 https://github.com/sudo-project/sudo/commit/bd209b9f16fcd127... Ah that makes a lot more sense. > So it's not just reading past the end of the buffer, but it's overwriting a single byte potentially belonging to another object. It may still cause a crash but it's relatively unlikely that it could cause something more severe. Yeah I agree that nothing severe should come from it. The allocations are probably larger then the size of the buffer being used so it may not even right off the end of its own allocation.
_notreallyme_ 4y ago
This might be exploitable in some cases. There has been a "heated" discussion in 2014 about off-by-one NUL byte heap overflow that lead to this blogpost from projectzero: https://googleprojectzero.blogspot.com/2014/08/the-poisoned-nul-byte-2014-edition.html https://googleprojectzero.blogspot.com/2014/08/the-poisoned-... There have been other examples where only a 1 could be written.
AtNightWeCode 4y ago
It is a very known exploit. Easily found with the stupid fuzz tests but even easier found with tests that tests all edge cases. You have to have a set of complete amateurs of coders to end up with this problem in production.
- AtNightWeCode 4y ago
  It is funny what you get downvoted for in this garbage forum. 15 years ago we built stuff in C and used cheap commercial available tools that detects EXACTLY this issue. There is no excuse to end up with this in prod 2022. Amateurs.
  - pjmlp 4y ago
    15 years? Lint was created in 1979 exactly to track down common errors in C, and until clang static analysis it was largely ignored. No wonder that these kind of tooling keeps being ignored until we get liability in software products.

dottedmag 4y ago

Probably only with severely niche libc. Is there any libc that allocates data with granularity smaller than 16 bytes?

nn3 4y ago
Very doubtful there is one. Seems like a total nothing burger
- tinus_hn 4y ago
  Sudo is a security boundary, it has to be rock solid and an issue that doesn’t immediately look exploitable is still a big deal. Sudo runs under the control of the attacker, it’s playing with fire!
  - ilyt 4y ago
    Case in point: the whole speculative execution was only suspected to maybe be exploitable decades ago, and only now we have a bunch of PoCs
    - userbinator 4y ago
      ...which are still only useful under highly contrived conditions which require knowing so much about the target that it wouldn't be a practical concern.

st_goliath 4y ago

> the seventh byte past the buffer is then zeroed/set to the null terminator. I wonder if and how this is exploitable. Well, looking at the upstream code, the original value is saved away before zeroing it out and then unconditionally restored after running crypt. If sudo is single threaded and crypt() doesn't malloc() anything, I don't think that can be exploited. Worst case would be a segfault if the password was somehow close enough to a page boundary.

geocar 4y ago
> I don't think that can be exploited. glibc malloc() should be aligned to 2*sizeof(size_t), so strup("")[x] on 64-bit systems (with 16-byte alignment) can never crash or overlap another object where x<16 On 32-bit systems and with other mallocs you could potentially be reaching another page (like I think you are imagining) or trashing some bookkeeping bits which might crash free() but I cannot yet see how you would induce that, nor convince myself it cannot be done without spending more time thinking about it (something I'm reluctant to do with my afternoon)

userbinator 4y ago

Seems like the buffer for storing the password was changed to be dynamically allocated When I see things like this, the first question I have is why? A password isn't going to be long enough to require dynamic allocation, so just use a fixed-size buffer. 255 is already generous and a good round number. The best solutions are often also the simplest.

AlexanderDhoore 4y ago
This is why I love working in embedded. Our clients would never complain about these kinds of reasonable restrictions. There are tons of limits like this in our machines, which nobody notices but make my life much easier. I guess things are different in the Linux/PC software world.
- tgv 4y ago
  It also might explain the S in IoT.
  - ilyt 4y ago
    I'd say not anymore, average microcontroller used in IoT got fat enough, nowadays even smaller chips come with AES acceleration too. I'm frankly surprised some proper standard didn't pop up already, I guess closing the users in your own ecosystem as fast as possible is priority in the industry. Good 80-90% of devices could be just "an MQTT connection + a bit of code to pair it up initially and feed server data" + a bunch of templates for how typical services should present themselves (so a light will just work with any compatible "control center"). Then just sell user subscription to your cloudy cloud IFTTT clone or a separate box to put in house that does the same job.
marcosdumay 4y ago
Because then you'll have another vulnerability created by too long passwords.
- GTP 4y ago

_kbh_ 4y ago

> I wonder if and how this is exploitable. Would be at worst a memory read past the inputted password, which wouldn't be super useful outside of a leak for another vulnerability but even that seems unlikely.

TonyTrapp 4y ago
No, the code overwrote the 9th byte of the buffer to add a null terminator: https://github.com/sudo-project/sudo/commit/bd209b9f16fcd1270c13db27ae3329c677d48050 https://github.com/sudo-project/sudo/commit/bd209b9f16fcd127... So it's not just reading past the end of the buffer, but it's overwriting a single byte potentially belonging to another object. It may still cause a crash but it's relatively unlikely that it could cause something more severe.
- Someone 4y ago
  I don’t understand how this can be correct: if (pw_len == DESLEN || HAS_AGEINFO(pw_epasswd, pw_len)) { strlcpy(des_pass, pass, sizeof(des_pass)); pass = des_pass; } Doesn’t that truncate your password if it happens to have DESLEN characters?
- _kbh_ 4y ago
  > No, the code overwrote the 9th byte of the buffer to add a null terminator: https://github.com/sudo-project/sudo/commit/bd209b9f16fcd127 https://github.com/sudo-project/sudo/commit/bd209b9f16fcd127... Ah that makes a lot more sense. > So it's not just reading past the end of the buffer, but it's overwriting a single byte potentially belonging to another object. It may still cause a crash but it's relatively unlikely that it could cause something more severe. Yeah I agree that nothing severe should come from it. The allocations are probably larger then the size of the buffer being used so it may not even right off the end of its own allocation.

_notreallyme_ 4y ago

This might be exploitable in some cases. There has been a "heated" discussion in 2014 about off-by-one NUL byte heap overflow that lead to this blogpost from projectzero: https://googleprojectzero.blogspot.com/2014/08/the-poisoned-nul-byte-2014-edition.html https://googleprojectzero.blogspot.com/2014/08/the-poisoned-... There have been other examples where only a 1 could be written.

AtNightWeCode 4y ago

It is a very known exploit. Easily found with the stupid fuzz tests but even easier found with tests that tests all edge cases. You have to have a set of complete amateurs of coders to end up with this problem in production.

AtNightWeCode 4y ago
It is funny what you get downvoted for in this garbage forum. 15 years ago we built stuff in C and used cheap commercial available tools that detects EXACTLY this issue. There is no excuse to end up with this in prod 2022. Amateurs.
- pjmlp 4y ago
  15 years? Lint was created in 1979 exactly to track down common errors in C, and until clang static analysis it was largely ignored. No wonder that these kind of tooling keeps being ignored until we get liability in software products.