Live data from Hacker News

Heap-based buffer overflow in Sudo

qualys.com

151–160 of 328 posts

Re: Heap-based buffer overflow in Sudo

#151

From 2017, there are at least 5 discovered security issues in sudo[1]. It seems a bit too untested for something running in root and interacting with all users. [1]: https://www.sudo.ws/

The kernel "runs as root and interacts with all users" and has a lot more than 5 discovered security issues ;)

Re: Heap-based buffer overflow in Sudo

#152
post #74

Earlier quoted context omitted.

Even if there were basic unit & regression tests, this bug might not have been caught. This bug should have gone through detailed security review and should probably also undergo fuzzing.

You can bet your bottom banana that the GRU, the NSA, Chinese state security, and the mob have all thoroughly fuzzed sudo and are sitting on the results. It just seems SO EASY to add a test for this problem, literally the relevant test input is one slash by itself, or any string ending in a slash! So simple! If I sent a change like this at work, no matter how trivial, that said it fixed this bug but I didn't send any…

Maybe expecting projects that mostly consist of a single guy working on it in their spare time to be "NSA proof" isn't really realistic?

Folk love to bring up "responsibility" and all of that, but you can't really expect people to bear the responsibility of the world on their shoulders for their spare time projects. It's neither realistic nor fair.

Re: Heap-based buffer overflow in Sudo

#153
post #59
post #12

Earlier quoted context omitted.

Why? After all it is obvious code reviewers are enough to catch any typical C memory corruption error.

Firstly, the constant nagging (of the kind appearing here, and often on other HN threads) against C and family are obnoxious. Any language will have both downsides and upsides and picking this one "flaw" in C and then pretending like it's a reason that C absolutely shouldn't be used is absurd. Secondly, the Sudo code in question seems to be a result of a poor or nonexistent design process (which also seems consistent…

On top of risks common to any language (which type systems are starting to work on), pointer arithmetic adds uniquely catastrophic risks that are no longer justified by big gains at runtime. People keep thinking they can get away with it, but we have decades of proof that they’re wrong. We can afford tools that don’t randomly explode.

Re: Heap-based buffer overflow in Sudo

#154
post #98

Earlier quoted context omitted.

That would indeed be a huge feat of programming skill! I refer you to the configuration file documentation to treat as a spec: https://www.sudo.ws/man/1.8.3/sudoers.man.html Implementing a subset of sudo is a weekend project, for some values of useful.

That's part of the dark outcome of so many untested features: it makes it easy to cast FUD upon any potential replacements. As we have no black-box test suite that shows `sudo` implements all of these features, so we also cannot have faith that any replacement would. Ideally it should be possible to run sudo's black box tests against any potential replacement. To start with, we need those tests.

There are actually a bunch of tests for the sudoers file format.

Re: Heap-based buffer overflow in Sudo

#155
post #74

Earlier quoted context omitted.

Even if there were basic unit & regression tests, this bug might not have been caught. This bug should have gone through detailed security review and should probably also undergo fuzzing.

You can bet your bottom banana that the GRU, the NSA, Chinese state security, and the mob have all thoroughly fuzzed sudo and are sitting on the results. It just seems SO EASY to add a test for this problem, literally the relevant test input is one slash by itself, or any string ending in a slash! So simple! If I sent a change like this at work, no matter how trivial, that said it fixed this bug but I didn't send any…

> It just seems SO EASY

Then do it! On your own time, rather than complaining that someone else didn't do it on theirs!

Re: Heap-based buffer overflow in Sudo

#156
post #152
post #74

Earlier quoted context omitted.

You can bet your bottom banana that the GRU, the NSA, Chinese state security, and the mob have all thoroughly fuzzed sudo and are sitting on the results. It just seems SO EASY to add a test for this problem, literally the relevant test input is one slash by itself, or any string ending in a slash! So simple! If I sent a change like this at work, no matter how trivial, that said it fixed this bug but I didn't send any…

Maybe expecting projects that mostly consist of a single guy working on it in their spare time to be "NSA proof" isn't really realistic? Folk love to bring up "responsibility" and all of that, but you can't really expect people to bear the responsibility of the world on their shoulders for their spare time projects. It's neither realistic nor fair.

No, but having tests is an acceptable baseline.

Re: Heap-based buffer overflow in Sudo

#157
post #156
post #152

Earlier quoted context omitted.

Maybe expecting projects that mostly consist of a single guy working on it in their spare time to be "NSA proof" isn't really realistic? Folk love to bring up "responsibility" and all of that, but you can't really expect people to bear the responsibility of the world on their shoulders for their spare time projects. It's neither realistic nor fair.

No, but having tests is an acceptable baseline.

There are tests. Are there enough tests? Maybe not. But people can do in their spare time whatever they want, including writing code without tests.

Re: Heap-based buffer overflow in Sudo

#158
post #63
post #57

All you need to know about sudo and frankly most other pieces of the Linux userspace is that it is undertested. The commit that added this flaw to sudo claims to fix a parser bug but includes no tests. There is no reason for the author, the reviewer (if there even was such a person), or anyone else to believe that the bug existed or was fixed by this change. The pull request that supposedly fixes this CVE also includ…

I think the design of the relevant code is worse than the lack of relevant test coverage. The problem solved insecurely by the code instead seems like an obvious target for lexical and syntactic analysis (and this has been so since the sixties, I think).

Yes, please at least use re2c when parsing anything more complicated in C. The result is much more readable, and integration costs are pretty low, and you still keep a lot of flexibility in how you structure your code.

Re: Heap-based buffer overflow in Sudo

#159
post #150

Earlier quoted context omitted.

> Despite this, the wisdom of the crowd is that you should never su to root, for ... reasons? `su` takes the password of the user you're becoming, while `sudo` takes the password (or not) of the user you already are. So using `su` to become root implies that there's a root password that multiple people (well, assuming there's multiple admins on the box) know.

sudo su My favorite command.

This is so silly that it's absolutely ludicrous, but I've never known about or used that before....... I can think of all kinds of other permutations of the command that I've used and know but not this one......

Re: Heap-based buffer overflow in Sudo

#160
post #45

Earlier quoted context omitted.

No, complexity strikes again. I haven't used sudo in years, preferring to use doas now. Its essential code is less than 500 lines and it does everything I've ever used sudo for, and that includes much more than `sudo `. $ man doas | wc -l 58 $ man doas.conf | wc -l 101 $ man sudo | wc -l 741 $ man sudoers | wc -l 3254 And a bonus: $ man sudoers | grep -C1 despair The sudoers file grammar will be described below in Ex…

But if sudo were written in Rust, it could have the same level of complexity and not be vulnerable. Yes, it would still be vulnerable to logic errors, like the last famous sudo bug where you pass -1 as the UID. But it wouldn't be vulnerable to this. (And this isn't the first memory safety bug to be found in sudo.) Yes, sudo's complexity is useless for 99.99% of its users. But wouldn't it be nice if the result were me…

Rust would have prevented the -1 as a UUID too, because you would have used a sum type (Rust enums) instead of a sigil there. Its easier, its idiomatic, its more clear, and the compiler knows how to optimize the overhead away a lot of the time.
Post reply on HN