Sudo: Heap-based overflow with small passwords (opens in new tab)

Todd is a saint for the work that he does here. Ultimately, like many other OpenBSD-related decisions, sudo was replaced by doas because of all the security-related issues with the code. Oftentimes, OpenBSD will replace a very functional solution, that has too many security issues, with one that has far less security issues and is, simply, good enough.

> All I want it to do is: ensure authorization, raise privileges, call exec.

A lot of people do want to do more, especially in some corporate contexts people run very complex sudo setups.

Consider something like Linode; maybe you want some of the more experienced support people to issue some commands on some machines, without having full control. And you've got a gazillion machines so you don't want to setup each one individually (as well as revoke access once they leave). Things can get fairly complex pretty quickly once you move out of simple settings.

For my desktop machine, doas is perfect. For our servers it's fine too because we have just a few people with access. But I'm not everyone of course.

doas was explicitly written to NOT cover all use cases, and that makes it better for the (simpler) use cases it was intended to solve, at the price of not covering other use cases of course.

That's the problem. We use sudo that autorizes user by it's SSH key via its auth socket... so we can essentially authenticate sudo via smartcard (Yubikey), and not keep the private key on user machine.

Some other folks use LDAP to get the sudoers files or even allowed ssh keys itself

There is also "just run thing as user" but also "set up same way shell would and pretend user logged as different user", first one is simple, second is a bunch of setup, copying env variables etc.

There is a good argument for splitting "just run app as different user" and "everything else that has to do with interactive shell and admins doing things on server" but now you have 2 configs to manage...

"ensure authorization" and "raise privileges" and "call exec" are all behind plugin-based architectures to allow for people to set up various ways to do things. The simple example: why do you only need to type in the password once per session/30 minutes calling sudo in most environments? How can people set things up with a hardware key? How do you make just some commands work and not others?

That being said I don't think sudo's code is complicated? It's big because it has all of these plugins supporting various mechanisms, but each file is pretty straightforward, and the control flow is really not that hard (after you get that it's plugins).

> All I want it to do is: ensure authorization, raise privileges, call exec.

And have an audit log? And clean the environment? In particular the path? Dynamic library path?

Still, I suspect "ensure autz" is the problem (implies Pam, 2fa, kerberos, etc etc).

Probably because of sudoers?

I also wonder....

    function sudo { su -c "$@"  root }

though I guess half the struggle is enabling sudo without a password

All of them.

Github does allow (some amount of) GH Actions for free, as does Gitlab.

Mh. I'm kinda sad that this is a distro patch. If that was upstream, we'd be using doas at this point in a few sideline setups, I'd guess. Plus, exploiting a directory list or writing files to a rather niche and secure directory generally requires rather high degrees of privilege and usually implies that the system is owned as a whole anyhow.

> EDIT: Oops, apparently Alpine added that

This seems like a really weird thing for a distro to add themselves rather than upstream.

> What typical use-cases do these particular features have?

For the io logs: auditing. And in some cases such logs are necessary for compliance reasons.

For wildcards: allowing to run a command, but only if they supply a specific option that makes it less dangerous. Now, you do have to be very careful when doing that, but there are cases where it is useful and explicitly listing every variation of options that is allowed isn't practical.

I also forgot to mention sudoedit. Although, making something similar for doas would be pretty simple, so I'm kind of surprised there isn't something.

For wildcards;

Allowing a user to use systemctl with specific daemons, so ```systemctl * unit-name```, without a password. But anything outside of that I need a password.

It also might explain the S in IoT.

With how many careless programmers there are, writing massive quantities of security-critical code...why would they bother? (Outside of a maybe a few narrow, high-value contexts.) That's like surreptitiously working to make sure that there are plenty of cat pictures on the web, and that water flows downhill.

As a default assumption, this may be a bit conspiratorial. Inserting something like this into a git repo is relatively easy to track, and a given "contributor" could not do many such things without being caught. Not saying we should ignore the possibility, though...

But there are plenty of ways for such agencies to gain similar access, including any kind of closed code in BIOSes, drivers, firmware etc. Or by taking control of select infra, and injecting MITM features there (that would remain stealthy, and only activate for very select targets.)

This should be a reasonable position to take. There's means and motivation present. The threat model should be taken seriously.

Not necessarily, as you can put a limit on how many characters you read from the shell. But you're right that you can screw up in both cases, I would just say that if you have a fixed length for the buffer it is a bit easier to handle it correctly.

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!

> No, the code overwrote the 9th byte of the buffer to add a null terminator: 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.

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?

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.

UNIX and C are symbiotic, for a long time, C would be the only compiler in the box, eventually C++ joined the party as they share the same origin.

Anything else required buying the product and justify why the compilers in the box wouldn't do it .

When UNIX SDKs became commercial, it was even worse, you would naturally one buy the main one, not pay twice for programming languages.

Then the UNIX clones also followed the same culture focusing on C for the clones. Early versions of the GNU manifesto explicitly refer that C should be the preferred language.

That is how we landed here.

Yes, I agree. I did not mean to imply that literally any other language would have been better for sudo, which I see is a viable reading of my original post. Go, for instance, would be a terrible choice, because the way the runtime deeply assumes you're running in a multithread environment, even before it gets to your "main" function, means that exactly the sort of UNIX hackery sudo is designed to do is effectively impossible. I have a system myself that is otherwise entirely in Go, but we have a very small C-based wrapper whose job it is to be setuid, open a few files with the escalated privileges, do some user verification, then change its uid and gid and exec the "real" Go program, because Go just can't do those things.

Dynamically typed languages as a whole would be a bad idea.

Java's startup time for such a small executable would be a problem.

I'm just saying this problem is unique to C, and in my opinion, sufficiently endemic to security software to disqualify it entirely.

Mind you, you might well end up at Rust in the end anyhow. Perhaps D. It isn't necessarily a long list for a sudo replacement. But...

C delenda est.

Do you know one of the reasons why Multics had a better security score than UNIX on DoD assement?

PL/I does bounds checking by default.

Like what? It would be written once correctly and work as is intended pretty much forever.

Rust came out in 2015, a year after the first release of Flatpak (back when it was still called xdg-app) in 2014. The ecosystem was also much smaller; some very necessary things for working nicely on Linux that exist now like zbus (the prominent async-compatible D-Bus library) wouldn't be a thing for several more years.

RH does have an interest in Rust, used in projects such as Stratis. It's just that the Linux dev ecosystem has been very C-reliant for a long time, and a massive amount of binding and other ecosystem work is still happening to make this possible.

*EDIT:* and the reason I mention Rust specifically is that, in these types of lower-level projects, a lot of things can start to get hairy very quickly in higher level languages. Things like some namespace APIs very much wanting to be run on a single thread, or trying to maintain performance when you're intercepting and examining every D-Bus message, or even just when you want your functionality to be in a reusable core library.

I understand why they stick to non-GC languages (performance, startup time, the need for maintained wrappers for native calls, etc.), but I don't understand why they don't pick better ones. Rust is nice and shiny but even modern C++ with some good conventions would be miles ahead of plain old C.

Languages like D even allow you to disable the garbage collector for specific methods, giving you the benefit of GC-less performance and characteristics in critical code paths and the YOLO memory management of GC languages in the wrappers around them.

I guess the answer is "because all the people over at Redhat know C"

You meant unsafe, not non-GC?

I'm pretty sure their point was that there are fundamental issues at play, and that switching languages does not make the problem go away.

Rust/whatever is not a magic bullet

For cli tools startup performance matters a lot, and many languages struggle on that point (eg Java) go and rust would probably be acceptable though.

It is, but that code wasn't added for the craic and does solve real use-cases that you can't just ignore for a full-featured drop-in s/sudo/.../-type replacement.

(also, 150k lines of code is a little bit misleading, since not all code is for all platforms, sudo has a plugin architecture I believe, etc.)

Changing ownership of /etc/ and directories under it like that sounds fine in theory but in practice breaks in many ways.

I did some extensive testing of this some years ago (on Debian/Ubuntu) and many system services and tools expect/require these directories to have specific ownership and permissions.

In the context I was experimenting with it was pretty simple too - renaming the UID 0 'root' account to some other name. That revealed that many tools actually test for "root" (the string) not uid == 0.

As I dug into the code of those tools I found many would also check and insist on particular ownership and modes on the directories and files.

I forget which one really annoyed me, but 'all' I wanted to do was allow members of group 'adm' to read/write into a particular sub-directory of /etc/ but the service would bail out if the directory wasn't owned by "root":"root" (or 0:0) and had 0700 permissions which is a pain when wanting to run services unprivileged and using 'setcap' to enable capabilities without starting as UID 0 and dropping privileges.

An alternate approach is to set the sysctl for this:

    net.ipv4.ip_unprivileged_port_start=80

Or whatever port you want unprivileged to start at. If you set it to 0 it means any user can bind to any port < 1024.

Ref: https://www.kernel.org/doc/htmldd/latest/networking/ip-sysct...

> Have an administrator `setcap cap_net_bind_service=+ep /your/binary/here` and you can use any port you want.

And remember to do it again every time the binary is updated :/

> chown /etc to nobody:wheel and

Bad idea! nobody is supposed to own no files at all. You run untrusted services (or untrusted users without account; something like anonymous FTP access) as nobody. This would potentially allow the least trusted entity to change your configs.

Apart from that. Since root can read any file anyways there is no reason to change the owner. And some programs may complain if the configuration is not owned by root.

Yes, having to always explicitly specify all the fine grained capabilities a process might need is a pain, too.

Having a process request its required capabilities and sudo displaying that list to the user, who can agree to sudo giving giving them only those capabilities would be good.

Unfortunately not. Pledge is _awesome_, but it's a different thing.

Pledge protects the system from buggy well-intentioned, cooperative software that could have bugs. What's needed is something that protects the system from ill-intentioned, uncooperative software.

IF you can execute code this way (which is an IF) then it's way more severe than a 2.9, and you could absolutely do anything you want with the system (you'll be root).

Complexity High isn't about what an attacker gets, it's about whether or not any specific configuration must exist for the attack to happen. For instance, if an app that talks to several services over different file transfer protocols has a vulnerability in only the FTP component, and these are not under attacker control, that's Complexity High.

Sure but you wouldn't be reading a password into a std::array or std::vector in C++, you'd be reading into a std::string or possibly something like a std::stringstream. And both of those containers will handle sizing and reallocation for you.

If your point is that C++ lets you do unsafe things then yes, of course it does. But so does Rust.