14 ms·

Sudo-rs' first security audit

62 points by Argorak 3y ago

g-b-r 3y ago
The arbitrary delete vulnerability scared me at first, but are there really situations where a non-root user can create a new user??
- mtlmtlmtlmtl 3y ago
  In practice, I doubt it. But sudo offers highly granular permission controls. You can permit a user to only run certain commands as root. So it is a real vuln in the sense that if you give someone that ability, you've now given them more than intended.
cedws 3y ago
I just ran tokei in the sudo-rs repository and there's over 28,000 lines of code not including whitespace. The Rust rewrite is a good step forward but we should really be asking ourselves if we need all this complexity in something so critical. OpenBSD's doas is 108 lines of C. sudo and doas are not equivalent in functionality, but it shows how simple things can really be. https://github.com/openbsd/src/blob/master/distrib/special/doas/doas.c https://github.com/openbsd/src/blob/master/distrib/special/d...
- eep_social 3y ago
  Which parts are you thinking of removing? The goal is “to build a drop-in replacement for all common use cases of sudo” so the doas comparison doesn’t really follow. The fabulous article mentions that, “sudo-rs only has 3 dependencies in its dependency graph” so maybe they could trade loc for deps but that doesn’t seem wise to me. The audit found one moderate path traversal vulnerability which was also present in og sudo, so I’m not sure how your suggestion could be made practical.
  - codetrotter 3y ago
    > the doas comparison doesn’t really follow The real question is, do you really need everything that sudo can do? Or would doas be sufficient? On my FreeBSD servers, I install doas instead of sudo, and I have never once found myself missing any features in spite of having completely replaced sudo with doas on my FreeBSD servers. Replaced as in, I no longer even have sudo installed on my FreeBSD servers. I switched from sudo to doas cold turkey years ago and haven’t looked back. (On Linux I still use sudo, but that is simply because it comes pre-installed on the Linux distros I use so I haven’t bothered to install doas instead there.) doas does exactly what I need; run the following command as root.
    - eep_social 3y ago
      Right, I’d speculate that removing sudo for doas is a heavy but feasible lift at the distro level. But as I said elsethread, I’m also very interested in an as-safe-as-possible replacement between now and then. Removing the need entirely (as they are both conceptually broken) seems like a huge lift that’s probably not feasible without fundamental changes. Could it be done within POSIX? IDK but I’d guess not.
      - cedws 3y ago
        >Removing the need entirely (as they are both conceptually broken) seems like a huge lift that’s probably not feasible without fundamental changes A sort of workaround is that you can log in as the desired user from a TTY. Of course, this gets tricky if you don't have physical access or a remote serial connection. And you probably wouldn't want to log in as root over SSH. I don't have real solutions in mind but it's ticking over in my head. Might have some ideas later on.
        eep_social 3y ago
        Aye, but then we are (I think) sharing credentials so we can both log in as the user with specific (read: elevated) permissions, and we lose any ability to know who the “real” person-user is on top. So it’s a different problem and we’re starting to talk about threat models and such..
- wrs 3y ago
  It would be great to have an educational effort to that effect: if you don’t need sudo, use doas. However, there are about six billion* things in existence right now that use sudo, so it’s a good idea to make sudo safer as well. * rhetorical statistic
- xerxes901 3y ago
  This doesn’t really change your conclusion, but I think that’s the wrong file. This is the real doas afaict: https://github.com/openbsd/src/blob/master/usr.bin/doas/doas.c https://github.com/openbsd/src/blob/master/usr.bin/doas/doas... Still just a tidy 1072 lines in that folder though. I spent 5 minutes staring at your file trying to understand how on earth it does the things in the man page, but of course it doesn’t.
  - cedws 3y ago
    Thanks, you're right. I was surprised how sparse it was.
- biorach 3y ago
  > we should really be asking ourselves if we need all this complexity in something so critical What is all the complexity? What is all the extra functionality that sudo offers?
  - cedws 3y ago
    Good question, never used it.
  - trclst 3y ago
    sudoedit would be an example.
  - Karellen 3y ago
    The ability to specify limited groups of commands that a subset of users can run, among other things. The "Examples" section of the sudoers(5) man page is probably a good place to start to get an idea of the sorts of ways it can be configured https://manpages.debian.org/bookworm/sudo/sudoers.5.en.html#EXAMPLES https://manpages.debian.org/bookworm/sudo/sudoers.5.en.html#...
  - ahefner 3y ago
    So I inherited a machine in our lab last week (not a "production" or "test" server, just a random machine being used as a dev environment) and decided to change its hostname using systemd's hostnamectl. Subsequently every attempt to execute a command via 'sudo' caused it to hang for multiple tens of seconds before declaring it couldn't resolve the new (nonexistent, entirely local) hostname. This persisted until I eventually fixed the /etc/hosts file which the systemd thing had failed to do, and I still don't know why sudo wanted to look up this hostname over the network, or what bad things could've happened if one of the configured DNS servers had maliciously replied for it.
    - g-b-r 3y ago
      Have you checked the sudoers?
- 0xbadcafebee 3y ago
  You can live a really simple life if you just stop using computers. Simple isn't always better.
- deleted 3y ago
  [deleted]
aliceryhl 3y ago
I'm surprised that CLN-003 made the list even as low severity. It's intended to make reverse engineering of the binary harder, but the code is already freely accessible (and CLN-003 also acknowledges this).
- thiht 3y ago
  I see it as a note to be exhaustive. It’s the kind of thing if you don’t add it to your report, some smart ass WILL say something like « actually they forgot about the bin symbols, how could they miss this? ». There’s always someone like this.
- RcouF1uZ4gsC 3y ago
  That vulnerability seems like something added to adhere to the rule of 3. In at least Western culture, we have this ingrained thing for groups of 3 - for example Trinity, 3 point outline, 3 sentences in a paragraph etc. It seems like this was picked to end up with 3 vulnerabilities so the security researchers can feel they did a complete job.
comex 3y ago
> Description: The cargo release build does not strip symbols, so they will be included in the final binary. (..) Impact: Since the code is open source, there is not much information to be gained, but removing these symbols might make reverse engineering of the binary harder. What a ridiculous finding. I can try to steelman the argument. Sure, maybe "reverse engineering of the binary" is useless most of the time for an open source project because you can just look at the source code. But if there were hypothetically a memory corruption vulnerability in sudo-rs, then an attacker would want to identify the specific machine code corresponding to the vulnerable source, in order to determine how it could be exploited. That wouldn't be too hard to achieve without symbols, but symbols would definitely make it easier. Except… even if the binary has symbols stripped, you can just `apt install sudo-rs-dbgsym`, or use debuginfod, to get the full debug info including symbols and DWARF records. Because distros provide that for all their packages. As a feature. To assist debugging. Even if distros didn't distribute debug symbols, today's security best practices include reproducible builds, which means you should be able to rebuild the package yourself and get the exact same binary, plus the symbols. So while it might save a tiny bit of disk space to strip the symbols, the security benefit is absolutely nil. …Well, in theory anyway. In practice, Debian's sudo-rs package seems to be missing both a dbgsym package and reproducible build info. But that's a bug!
- Smaug123 3y ago
  That is what "low" means as a vulnerability severity! "Low" means "you totally don't need to do anything about this". The reported property does make the attacker's job slightly harder, after all, since they need to go and work out where the symbols are rather than just having them right there in front of them.
  - comex 3y ago
    The problem with the report isn’t just that it’s minor. The problem is that it’s directionally wrong. If the change makes life even slightly harder for attackers, then it makes life harder to exactly the same extent for legitimate users trying to debug their programs. The fact that debuginfo is widely distributed demonstrates that distros prioritize the benefits of debuggability over the benefits of obscurity, and there’s no reason that weighting should be different in the small versus in the large. So, if security and debuggability were the only concerns, debuggability should win and distro binaries should never be stripped. It just so happens that there is an unrelated concern, disk space, that justifies stripping them anyway, but that doesn’t justify characterizing the lack of stripping as a security flaw, not even an incredibly minor one.
0xbadcafebee 3y ago
CLN-001: relative path traversal vulnerability (moderate) During the audit, it came to light that the original sudo implementation was also affected by this issue, although with a lower security severity due to their use of the openat function. I thought Rust was secure? How is it possible to write a program in Rust and still have the same security vulnerabilities, and actually be higher severity? It's almost as if changing to an entirely new programming language and ecosystem isn't enough to make a secure application, and that you still have to try hard to secure it, regardless of the language. How interesting.
- dataking 3y ago
  This is a logic error whereas Rust delivers a way to drive down memory errors (assuming parent comment isn't mocking Rust advocates).
- deleted 3y ago
  [deleted]
- ncallaway 3y ago
  > It's almost as if changing to an entirely new programming language and ecosystem isn't enough to make a secure application, and that you still have to try hard to secure it, regardless of the language. Has anyone argued that switching to Rust alone is sufficient to stop all security vulnerabilities? Has anyone suggested that switching to Rust means you no longer need to do any work to write secure applications? It seems to me you're arguing against a totally self-constructed straw-man.
- viraptor 3y ago
  That's a low quality troll. Nobody was saying rust solves all security issues.
  - 0xbadcafebee 3y ago
    It's hilarious that people invented an entirely new language to prevent a single attack vector. That's like learning Perl because it has no buffer overflows.

62 points by Argorak 3y ago

The arbitrary delete vulnerability scared me at first, but are there really situations where a non-root user can create a new user??

mtlmtlmtlmtl 3y ago
In practice, I doubt it. But sudo offers highly granular permission controls. You can permit a user to only run certain commands as root. So it is a real vuln in the sense that if you give someone that ability, you've now given them more than intended.

cedws 3y ago

I just ran tokei in the sudo-rs repository and there's over 28,000 lines of code not including whitespace. The Rust rewrite is a good step forward but we should really be asking ourselves if we need all this complexity in something so critical. OpenBSD's doas is 108 lines of C. sudo and doas are not equivalent in functionality, but it shows how simple things can really be. https://github.com/openbsd/src/blob/master/distrib/special/doas/doas.c https://github.com/openbsd/src/blob/master/distrib/special/d...

eep_social 3y ago
Which parts are you thinking of removing? The goal is “to build a drop-in replacement for all common use cases of sudo” so the doas comparison doesn’t really follow. The fabulous article mentions that, “sudo-rs only has 3 dependencies in its dependency graph” so maybe they could trade loc for deps but that doesn’t seem wise to me. The audit found one moderate path traversal vulnerability which was also present in og sudo, so I’m not sure how your suggestion could be made practical.
- codetrotter 3y ago
  > the doas comparison doesn’t really follow The real question is, do you really need everything that sudo can do? Or would doas be sufficient? On my FreeBSD servers, I install doas instead of sudo, and I have never once found myself missing any features in spite of having completely replaced sudo with doas on my FreeBSD servers. Replaced as in, I no longer even have sudo installed on my FreeBSD servers. I switched from sudo to doas cold turkey years ago and haven’t looked back. (On Linux I still use sudo, but that is simply because it comes pre-installed on the Linux distros I use so I haven’t bothered to install doas instead there.) doas does exactly what I need; run the following command as root.
  - eep_social 3y ago
    Right, I’d speculate that removing sudo for doas is a heavy but feasible lift at the distro level. But as I said elsethread, I’m also very interested in an as-safe-as-possible replacement between now and then. Removing the need entirely (as they are both conceptually broken) seems like a huge lift that’s probably not feasible without fundamental changes. Could it be done within POSIX? IDK but I’d guess not.
    - cedws 3y ago
      >Removing the need entirely (as they are both conceptually broken) seems like a huge lift that’s probably not feasible without fundamental changes A sort of workaround is that you can log in as the desired user from a TTY. Of course, this gets tricky if you don't have physical access or a remote serial connection. And you probably wouldn't want to log in as root over SSH. I don't have real solutions in mind but it's ticking over in my head. Might have some ideas later on.
      - eep_social 3y ago
        Aye, but then we are (I think) sharing credentials so we can both log in as the user with specific (read: elevated) permissions, and we lose any ability to know who the “real” person-user is on top. So it’s a different problem and we’re starting to talk about threat models and such..
wrs 3y ago
It would be great to have an educational effort to that effect: if you don’t need sudo, use doas. However, there are about six billion* things in existence right now that use sudo, so it’s a good idea to make sudo safer as well. * rhetorical statistic
xerxes901 3y ago
This doesn’t really change your conclusion, but I think that’s the wrong file. This is the real doas afaict: https://github.com/openbsd/src/blob/master/usr.bin/doas/doas.c https://github.com/openbsd/src/blob/master/usr.bin/doas/doas... Still just a tidy 1072 lines in that folder though. I spent 5 minutes staring at your file trying to understand how on earth it does the things in the man page, but of course it doesn’t.
- cedws 3y ago
  Thanks, you're right. I was surprised how sparse it was.
biorach 3y ago
> we should really be asking ourselves if we need all this complexity in something so critical What is all the complexity? What is all the extra functionality that sudo offers?
- cedws 3y ago
  Good question, never used it.
- trclst 3y ago
  sudoedit would be an example.
- Karellen 3y ago
  The ability to specify limited groups of commands that a subset of users can run, among other things. The "Examples" section of the sudoers(5) man page is probably a good place to start to get an idea of the sorts of ways it can be configured https://manpages.debian.org/bookworm/sudo/sudoers.5.en.html#EXAMPLES https://manpages.debian.org/bookworm/sudo/sudoers.5.en.html#...
- ahefner 3y ago
  So I inherited a machine in our lab last week (not a "production" or "test" server, just a random machine being used as a dev environment) and decided to change its hostname using systemd's hostnamectl. Subsequently every attempt to execute a command via 'sudo' caused it to hang for multiple tens of seconds before declaring it couldn't resolve the new (nonexistent, entirely local) hostname. This persisted until I eventually fixed the /etc/hosts file which the systemd thing had failed to do, and I still don't know why sudo wanted to look up this hostname over the network, or what bad things could've happened if one of the configured DNS servers had maliciously replied for it.
  - g-b-r 3y ago
    Have you checked the sudoers?
0xbadcafebee 3y ago
You can live a really simple life if you just stop using computers. Simple isn't always better.
deleted 3y ago
[deleted]

aliceryhl 3y ago

I'm surprised that CLN-003 made the list even as low severity. It's intended to make reverse engineering of the binary harder, but the code is already freely accessible (and CLN-003 also acknowledges this).

thiht 3y ago
I see it as a note to be exhaustive. It’s the kind of thing if you don’t add it to your report, some smart ass WILL say something like « actually they forgot about the bin symbols, how could they miss this? ». There’s always someone like this.
RcouF1uZ4gsC 3y ago
That vulnerability seems like something added to adhere to the rule of 3. In at least Western culture, we have this ingrained thing for groups of 3 - for example Trinity, 3 point outline, 3 sentences in a paragraph etc. It seems like this was picked to end up with 3 vulnerabilities so the security researchers can feel they did a complete job.

comex 3y ago

> Description: The cargo release build does not strip symbols, so they will be included in the final binary. (..) Impact: Since the code is open source, there is not much information to be gained, but removing these symbols might make reverse engineering of the binary harder. What a ridiculous finding. I can try to steelman the argument. Sure, maybe "reverse engineering of the binary" is useless most of the time for an open source project because you can just look at the source code. But if there were hypothetically a memory corruption vulnerability in sudo-rs, then an attacker would want to identify the specific machine code corresponding to the vulnerable source, in order to determine how it could be exploited. That wouldn't be too hard to achieve without symbols, but symbols would definitely make it easier. Except… even if the binary has symbols stripped, you can just `apt install sudo-rs-dbgsym`, or use debuginfod, to get the full debug info including symbols and DWARF records. Because distros provide that for all their packages. As a feature. To assist debugging. Even if distros didn't distribute debug symbols, today's security best practices include reproducible builds, which means you should be able to rebuild the package yourself and get the exact same binary, plus the symbols. So while it might save a tiny bit of disk space to strip the symbols, the security benefit is absolutely nil. …Well, in theory anyway. In practice, Debian's sudo-rs package seems to be missing both a dbgsym package and reproducible build info. But that's a bug!

Smaug123 3y ago
That is what "low" means as a vulnerability severity! "Low" means "you totally don't need to do anything about this". The reported property does make the attacker's job slightly harder, after all, since they need to go and work out where the symbols are rather than just having them right there in front of them.
- comex 3y ago
  The problem with the report isn’t just that it’s minor. The problem is that it’s directionally wrong. If the change makes life even slightly harder for attackers, then it makes life harder to exactly the same extent for legitimate users trying to debug their programs. The fact that debuginfo is widely distributed demonstrates that distros prioritize the benefits of debuggability over the benefits of obscurity, and there’s no reason that weighting should be different in the small versus in the large. So, if security and debuggability were the only concerns, debuggability should win and distro binaries should never be stripped. It just so happens that there is an unrelated concern, disk space, that justifies stripping them anyway, but that doesn’t justify characterizing the lack of stripping as a security flaw, not even an incredibly minor one.

0xbadcafebee 3y ago

CLN-001: relative path traversal vulnerability (moderate) During the audit, it came to light that the original sudo implementation was also affected by this issue, although with a lower security severity due to their use of the openat function. I thought Rust was secure? How is it possible to write a program in Rust and still have the same security vulnerabilities, and actually be higher severity? It's almost as if changing to an entirely new programming language and ecosystem isn't enough to make a secure application, and that you still have to try hard to secure it, regardless of the language. How interesting.

dataking 3y ago
This is a logic error whereas Rust delivers a way to drive down memory errors (assuming parent comment isn't mocking Rust advocates).
deleted 3y ago
[deleted]
ncallaway 3y ago
> It's almost as if changing to an entirely new programming language and ecosystem isn't enough to make a secure application, and that you still have to try hard to secure it, regardless of the language. Has anyone argued that switching to Rust alone is sufficient to stop all security vulnerabilities? Has anyone suggested that switching to Rust means you no longer need to do any work to write secure applications? It seems to me you're arguing against a totally self-constructed straw-man.
viraptor 3y ago
That's a low quality troll. Nobody was saying rust solves all security issues.
- 0xbadcafebee 3y ago
  It's hilarious that people invented an entirely new language to prevent a single attack vector. That's like learning Perl because it has no buffer overflows.