8 ms·

Sudoedit can edit arbitrary files

109 points by accessvector 4y ago

syrrim 4y ago
Is there a patch, or more detailed explanation of what causes this?
- slaymaker1907 4y ago
  There's a detailed writeup mentioned in the post https://www.synacktiv.com/sites/default/files/2023-01/sudo-CVE-2023-22809.pdf https://www.synacktiv.com/sites/default/files/2023-01/sudo-C....
- nequo 4y ago
  Ubuntu shipped the patch three days ago. The output of `apt changelog sudo` on 22.04 LTS: sudo (1.9.9-1ubuntu2.2) jammy-security; urgency=medium * SECURITY UPDATE: arbitrary file overwrite via sudoedit - debian/patches/CVE-2023-22809.patch: do not permit editor arguments to include -- in plugins/sudoers/editor.c, plugins/sudoers/sudoers.c, plugins/sudoers/visudo.c. - CVE-2023-22809 * SECURITY UPDATE: DoS via invalid arithmetic shift in Protobuf-c - debian/patches/CVE-2022-33070.patch: only shift unsigned values in lib/protobuf-c/protobuf-c.c. - CVE-2022-33070 -- Marc Deslauriers <marc.deslauriers@ubuntu.com> Mon, 16 Jan 2023 07:36:33 -0500 There is a detailed explanation on the sudo website: https://www.sudo.ws/security/advisories/sudoedit_any/ https://www.sudo.ws/security/advisories/sudoedit_any/
- asveikau 4y ago
  It shells out to the EDITOR environment variable, which is controlled by the less privileged user. In this example they inject running an editor against another file. I'm guessing you can put arbitrary code in there or point it at a locally controlled executable too. But I'm not sure. Maybe sudoedit puts more scrutiny on that variable than most, non-security programs. At any rate many text editors have lots of modules and scripting and can presumably load and execute code as the privileged user. The workaround is to change the sudo config file to remove the EDITOR environment variable and a few others.
  - zokier 4y ago
    > At any rate many text editors have lots of modules and scripting and can presumably load and execute code as the privileged user. Sudoedit does not run the editor as privileged user, that is kinda the whole point
stabbles 4y ago
I wonder if this bug in logic (instead of buffer overflows) would also have been less likely in a different language. Would it have been more obvious in a language where it's easier to work with dynamically allocated arrays and strings?
- mattpallissard 4y ago
  Doubtful, failing to sanitize your inputs plagues memory safe languages too.
  - deafpolygon 4y ago
    It's kind of tiring to hear about memory safe languages (mainly rust here) being put on a pedestal, as if it will solve all our software woes. Not to mention, C++ /can/ be memory safe if you use memory-safe routines. I'm just waiting for articles to come in, this year.. "why 2023 was not the year of Rust". Not hating on Rust - just the evangelism.
- dllthomas 4y ago
  I don't see a change to language, per se, that would have helped, really. A system with more of an object capabilities model could have helped, though. The goal wasn't really "let the user run their editor as root (when they ask for it)", but "let the user work with this particular file from their editor (when they ask for it)".
- JoshTriplett 4y ago
  With my Rust hat on: I don't think that Rust would have solved this. It might have made the code in question easier to understand, as you note, but this kind of error can still happen in any language.
- arp242 4y ago
  Looking at the patch[1], probably not. There isn't really a lot of complex string handling involved; it's basically just forgetting to forbid "--". I don't really see how any language choice could help you with this. [1]: https://github.com/sudo-project/sudo/commit/0274a4f3b403162a37a10f199c989f3727ed3ad4 https://github.com/sudo-project/sudo/commit/0274a4f3b403162a...
dejj 4y ago
Why is this a problem, given that one can easily use sudoedit for privilege escalation already? edit: I now realize I have confused sudoedit with visudo
- jboy55 4y ago
  I read it at first take as if it was "CVE-2023-32049: 'su' has critical privilege escalation venerability"
- arp242 4y ago
  Read the link instead of replying based on the title.
binkHN 4y ago
I moved to https://man.openbsd.org/doas https://man.openbsd.org/doas long ago.
- deleted 4y ago
  [deleted]
- shashasha2 4y ago
  Is doas safe about this exploit or does it have the same problem ?
  - yencabulator 4y ago
    doas has no "doasedit". It's a lot simpler than sudo, on purpose.
michalsustr 4y ago
Why would one prefer to add sudoedit X to sudoers rather than updating file access privileges of X directly? Just curious about arguments for this use case.
- throw0101c 4y ago
  > Why would one prefer to add sudoedit X to sudoers rather than updating file access privileges of X directly? Permission complications. Software may run as user:group, but you don't want to add humans to either, and so you allow them to edit a few files as that user or group from their own account (which also gives you auditing of changes). Some software insists on files (directories) have certain permissions so you're stuff with them. Or you want a centralized place for permissions, so you put these sudoedit entries in LDAP which can be accessed anywhere in you network, and so you don't have to keep track of individual file permissions on a gazillion systems.
- eklitzke 4y ago
  Sudo basically has an ACL-like system where you can specify exactly which users/groups can execute which commands as root. So you can say user foo can execute commands X, Y, and Z as root and user bar can execute commands W, Y, and Z as root, and neither user can use sudo to execute any other command as root. The ACL system isn't for sudoedit specifically, it's a general feature of sudo. As to why you can't just update access privileges of the file, for most use cases you probably could do that. If you need something more complicated though you'll have to use some terrible ACL implementation like the one in sudo or Posix file ACLs.
  - mananaysiempre 4y ago
    I recently got to reading the POSIX.1e (MAC & DAC) draft, and the DAC = ACL part is... surprisingly non-terrible. Still awkward and hampered by its existence as barely-visible metadata smeared over the whole system, as all ACLs are, but not at all the hopeless mess I expected coming from NT. (Even that might’ve been salvageable had Microsoft been willing to publish full documentation of all NT object permissions and mechanisms. Except SDDL, there is no world in which SDDL is salvageable.) Couldn’t make heads or tails of the MAC part, though. The /etc/sudoers solution does have a usability advantage precisely in not being smeared all over the system. Even if “/etc/sudoers” and “usability” are words not often seen inside a single sentence.
throw0101c 4y ago
I find it handy that most distros have a CVE look-up 'service': * https://security-tracker.debian.org/tracker/CVE-2023-22809 https://security-tracker.debian.org/tracker/CVE-2023-22809 * https://ubuntu.com/security/CVE-2023-22809 https://ubuntu.com/security/CVE-2023-22809 * https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2023-22809 https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2023-22809 Debian has links to the others.
- sva_ 4y ago
  https://security.archlinux.org/ https://security.archlinux.org/
  - asddgsgsaxg 4y ago
    [dead]
- mroche 4y ago
  I would opt for the Red Hat CVE Database over Bugzilla if you aren't using Fedora, but they all interlink: https://access.redhat.com/security/cve/CVE-2023-22809 https://access.redhat.com/security/cve/CVE-2023-22809 Red Hat has quite a few services under the /security path.
- _ikke_ 4y ago
  https://security.alpinelinux.org/vuln/CVE-2023-22809 https://security.alpinelinux.org/vuln/CVE-2023-22809
tinus_hn 4y ago
Does this really work? The command is supposed to copy the original file to a temporary file, run the edit command with the privileges of the original user and then copy the edited file over the original. Otherwise what’s stopping an attacker from telling the editor to just open another file?
- eklitzke 4y ago
  You're correct but sudoedit itself needs to parse the file list to know which files to copy to temporary files as you describe. So in this case you're tricking sudoedit into thinking you want to edit a different file than the one specified originally on the command line.
- DSMan195276 4y ago
  Yeah I had the same confusion, the linked PDF explains it. Basically sudo determines the list of files to edit after expanding the `EDITOR` variable into separate arguments, and the `--` in the argument list (added by `sudo`) is used to determine where the file arguments provided to `sudoedit` start in the new argument list. By adding your own `--` in the `EDITOR` variable, `sudo` gets confused and thinks that `--` is the start of the `sudoedit` file arguments and thus happily copies and edits all the files after it.
  - tinus_hn 4y ago
    Incredible! So the problem is not -- but the problem is that it is checking the wrong thing to begin with. Why even parse the string, sudo already had the list of files when it constructed the string..
    - DSMan195276 4y ago
      I mean I agree, I'd say it's mostly just an issue of too much separation, they put the argument array together in one piece of code and then pass it to another piece of code that executes it with the necessary permissions. They don't pass along a separate array of files (or the location in the arguments where the files start), so the execute code attempts to figure out where they are instead.
inopinatus 4y ago
Consider this a prompt to review your /etc/sudoers for any utility whose behaviour is modified by environment variables in the env_keep list.
- remram 4y ago
  I don't think that's what's happening. sudoedit does NOT run the editor as root, it copies the file to a temporary as root, runs the editor as you, and copies the temporary over the target file as root when you're done editing it (at least it's supposed to).
  - inopinatus 4y ago
    Looks like a misclick, but unsure which comment was intended for this reply.
    - remram 4y ago
      Yours. The editor cannot be tricked into editing the wrong file as root by environment variables, because it is not running as root. The security is an actual flaw in sudoedit, the wrapper script, not a fundamental issue with the environment you pass to the command.
      - inopinatus 4y ago
        I did not mention an editor, so that reading doesn’t follow. Other comments certainly did and could be wrong in the fashion you describe. However, I did not write them, so I feel no need to defend them. My point is simply the actionable generalisation of your followup, the substantive part of which I don’t even disagree with. In this instance the utility is the wrapper.
orf 4y ago
The fix: https://github.com/sudo-project/sudo/commit/0274a4f3b403162a37a10f199c989f3727ed3ad4 https://github.com/sudo-project/sudo/commit/0274a4f3b403162a...
deleted 4y ago
[deleted]

109 points by accessvector 4y ago

syrrim 4y ago

Is there a patch, or more detailed explanation of what causes this?

slaymaker1907 4y ago
There's a detailed writeup mentioned in the post https://www.synacktiv.com/sites/default/files/2023-01/sudo-CVE-2023-22809.pdf https://www.synacktiv.com/sites/default/files/2023-01/sudo-C....
nequo 4y ago
Ubuntu shipped the patch three days ago. The output of `apt changelog sudo` on 22.04 LTS: sudo (1.9.9-1ubuntu2.2) jammy-security; urgency=medium * SECURITY UPDATE: arbitrary file overwrite via sudoedit - debian/patches/CVE-2023-22809.patch: do not permit editor arguments to include -- in plugins/sudoers/editor.c, plugins/sudoers/sudoers.c, plugins/sudoers/visudo.c. - CVE-2023-22809 * SECURITY UPDATE: DoS via invalid arithmetic shift in Protobuf-c - debian/patches/CVE-2022-33070.patch: only shift unsigned values in lib/protobuf-c/protobuf-c.c. - CVE-2022-33070 -- Marc Deslauriers <marc.deslauriers@ubuntu.com> Mon, 16 Jan 2023 07:36:33 -0500 There is a detailed explanation on the sudo website: https://www.sudo.ws/security/advisories/sudoedit_any/ https://www.sudo.ws/security/advisories/sudoedit_any/
asveikau 4y ago
It shells out to the EDITOR environment variable, which is controlled by the less privileged user. In this example they inject running an editor against another file. I'm guessing you can put arbitrary code in there or point it at a locally controlled executable too. But I'm not sure. Maybe sudoedit puts more scrutiny on that variable than most, non-security programs. At any rate many text editors have lots of modules and scripting and can presumably load and execute code as the privileged user. The workaround is to change the sudo config file to remove the EDITOR environment variable and a few others.
- zokier 4y ago
  > At any rate many text editors have lots of modules and scripting and can presumably load and execute code as the privileged user. Sudoedit does not run the editor as privileged user, that is kinda the whole point

stabbles 4y ago

I wonder if this bug in logic (instead of buffer overflows) would also have been less likely in a different language. Would it have been more obvious in a language where it's easier to work with dynamically allocated arrays and strings?

mattpallissard 4y ago
Doubtful, failing to sanitize your inputs plagues memory safe languages too.
- deafpolygon 4y ago
  It's kind of tiring to hear about memory safe languages (mainly rust here) being put on a pedestal, as if it will solve all our software woes. Not to mention, C++ /can/ be memory safe if you use memory-safe routines. I'm just waiting for articles to come in, this year.. "why 2023 was not the year of Rust". Not hating on Rust - just the evangelism.
dllthomas 4y ago
I don't see a change to language, per se, that would have helped, really. A system with more of an object capabilities model could have helped, though. The goal wasn't really "let the user run their editor as root (when they ask for it)", but "let the user work with this particular file from their editor (when they ask for it)".
JoshTriplett 4y ago
With my Rust hat on: I don't think that Rust would have solved this. It might have made the code in question easier to understand, as you note, but this kind of error can still happen in any language.
arp242 4y ago
Looking at the patch[1], probably not. There isn't really a lot of complex string handling involved; it's basically just forgetting to forbid "--". I don't really see how any language choice could help you with this. [1]: https://github.com/sudo-project/sudo/commit/0274a4f3b403162a37a10f199c989f3727ed3ad4 https://github.com/sudo-project/sudo/commit/0274a4f3b403162a...

dejj 4y ago

Why is this a problem, given that one can easily use sudoedit for privilege escalation already? edit: I now realize I have confused sudoedit with visudo

jboy55 4y ago
I read it at first take as if it was "CVE-2023-32049: 'su' has critical privilege escalation venerability"
arp242 4y ago
Read the link instead of replying based on the title.

binkHN 4y ago

I moved to https://man.openbsd.org/doas https://man.openbsd.org/doas long ago.

deleted 4y ago
[deleted]
shashasha2 4y ago
Is doas safe about this exploit or does it have the same problem ?
- yencabulator 4y ago
  doas has no "doasedit". It's a lot simpler than sudo, on purpose.

michalsustr 4y ago

Why would one prefer to add sudoedit X to sudoers rather than updating file access privileges of X directly? Just curious about arguments for this use case.

throw0101c 4y ago
> Why would one prefer to add sudoedit X to sudoers rather than updating file access privileges of X directly? Permission complications. Software may run as user:group, but you don't want to add humans to either, and so you allow them to edit a few files as that user or group from their own account (which also gives you auditing of changes). Some software insists on files (directories) have certain permissions so you're stuff with them. Or you want a centralized place for permissions, so you put these sudoedit entries in LDAP which can be accessed anywhere in you network, and so you don't have to keep track of individual file permissions on a gazillion systems.
eklitzke 4y ago
Sudo basically has an ACL-like system where you can specify exactly which users/groups can execute which commands as root. So you can say user foo can execute commands X, Y, and Z as root and user bar can execute commands W, Y, and Z as root, and neither user can use sudo to execute any other command as root. The ACL system isn't for sudoedit specifically, it's a general feature of sudo. As to why you can't just update access privileges of the file, for most use cases you probably could do that. If you need something more complicated though you'll have to use some terrible ACL implementation like the one in sudo or Posix file ACLs.
- mananaysiempre 4y ago
  I recently got to reading the POSIX.1e (MAC & DAC) draft, and the DAC = ACL part is... surprisingly non-terrible. Still awkward and hampered by its existence as barely-visible metadata smeared over the whole system, as all ACLs are, but not at all the hopeless mess I expected coming from NT. (Even that might’ve been salvageable had Microsoft been willing to publish full documentation of all NT object permissions and mechanisms. Except SDDL, there is no world in which SDDL is salvageable.) Couldn’t make heads or tails of the MAC part, though. The /etc/sudoers solution does have a usability advantage precisely in not being smeared all over the system. Even if “/etc/sudoers” and “usability” are words not often seen inside a single sentence.

throw0101c 4y ago

I find it handy that most distros have a CVE look-up 'service': * https://security-tracker.debian.org/tracker/CVE-2023-22809 https://security-tracker.debian.org/tracker/CVE-2023-22809 * https://ubuntu.com/security/CVE-2023-22809 https://ubuntu.com/security/CVE-2023-22809 * https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2023-22809 https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2023-22809 Debian has links to the others.

sva_ 4y ago
https://security.archlinux.org/ https://security.archlinux.org/
- asddgsgsaxg 4y ago
  [dead]
mroche 4y ago
I would opt for the Red Hat CVE Database over Bugzilla if you aren't using Fedora, but they all interlink: https://access.redhat.com/security/cve/CVE-2023-22809 https://access.redhat.com/security/cve/CVE-2023-22809 Red Hat has quite a few services under the /security path.
_ikke_ 4y ago
https://security.alpinelinux.org/vuln/CVE-2023-22809 https://security.alpinelinux.org/vuln/CVE-2023-22809

tinus_hn 4y ago

Does this really work? The command is supposed to copy the original file to a temporary file, run the edit command with the privileges of the original user and then copy the edited file over the original. Otherwise what’s stopping an attacker from telling the editor to just open another file?

eklitzke 4y ago
You're correct but sudoedit itself needs to parse the file list to know which files to copy to temporary files as you describe. So in this case you're tricking sudoedit into thinking you want to edit a different file than the one specified originally on the command line.
DSMan195276 4y ago
Yeah I had the same confusion, the linked PDF explains it. Basically sudo determines the list of files to edit after expanding the `EDITOR` variable into separate arguments, and the `--` in the argument list (added by `sudo`) is used to determine where the file arguments provided to `sudoedit` start in the new argument list. By adding your own `--` in the `EDITOR` variable, `sudo` gets confused and thinks that `--` is the start of the `sudoedit` file arguments and thus happily copies and edits all the files after it.
- tinus_hn 4y ago
  Incredible! So the problem is not -- but the problem is that it is checking the wrong thing to begin with. Why even parse the string, sudo already had the list of files when it constructed the string..
  - DSMan195276 4y ago
    I mean I agree, I'd say it's mostly just an issue of too much separation, they put the argument array together in one piece of code and then pass it to another piece of code that executes it with the necessary permissions. They don't pass along a separate array of files (or the location in the arguments where the files start), so the execute code attempts to figure out where they are instead.

inopinatus 4y ago

Consider this a prompt to review your /etc/sudoers for any utility whose behaviour is modified by environment variables in the env_keep list.

remram 4y ago
I don't think that's what's happening. sudoedit does NOT run the editor as root, it copies the file to a temporary as root, runs the editor as you, and copies the temporary over the target file as root when you're done editing it (at least it's supposed to).
- inopinatus 4y ago
  Looks like a misclick, but unsure which comment was intended for this reply.
  - remram 4y ago
    Yours. The editor cannot be tricked into editing the wrong file as root by environment variables, because it is not running as root. The security is an actual flaw in sudoedit, the wrapper script, not a fundamental issue with the environment you pass to the command.
    - inopinatus 4y ago
      I did not mention an editor, so that reading doesn’t follow. Other comments certainly did and could be wrong in the fashion you describe. However, I did not write them, so I feel no need to defend them. My point is simply the actionable generalisation of your followup, the substantive part of which I don’t even disagree with. In this instance the utility is the wrapper.

orf 4y ago

The fix: https://github.com/sudo-project/sudo/commit/0274a4f3b403162a37a10f199c989f3727ed3ad4 https://github.com/sudo-project/sudo/commit/0274a4f3b403162a...

deleted 4y ago

[deleted]