LWN: Comments on "A major vulnerability in Sudo"

A major vulnerability in Sudo

mina86 — Tue, 23 Feb 2021 10:45:14 +0000

Hmm… I stand corrected. (Though creating an array just to iterate over characters feels wasteful).

A major vulnerability in Sudo

anselm — Tue, 23 Feb 2021 10:26:22 +0000

What's wrong with

foreach (split //, "abcde") {
   …
}

A major vulnerability in Sudo

immibis — Wed, 17 Feb 2021 14:58:44 +0000

How does one elevate privilege in Qubes, and why is it immune to bugs?

A major vulnerability in Sudo

mina86 — Wed, 17 Feb 2021 12:11:00 +0000

> What languages make it hard to iterate through the characters of a string? I can't think of a single one.

Perl. ;)

A major vulnerability in Sudo

nix — Thu, 11 Feb 2021 17:43:10 +0000

Via a fifo named /dev/mp3, and this exciting daemon:

#!/usr/bin/zsh
FIFO=/dev/mp3

[[ ! -p $FIFO ]] && { echo "Error: $FIFO does not exist or is not a FIFO." >&2; exit 1; }

while true; do mpg123 -b 1024 - < $FIFO >/dev/null 2>/dev/null; done

(This is decades old. If I was doing it these days, I'd probably not pick zsh at random and I'd probably use gst123 so it could play anything, not just mp3s. Obviously this needs a systemwide PulseAudio or something similar: again, this script predates PulseAudio entirely :) )

A major vulnerability in Sudo

linuxrocks123 — Thu, 11 Feb 2021 04:47:09 +0000

How does that work?

userv as alternative

ber — Mon, 08 Feb 2021 09:23:21 +0000

Probably
https://packages.debian.org/buster/userv
http://www.chiark.greenend.org.uk/~ian/userv/
is, what Werner refers to

A major vulnerability in Sudo

rcampos — Sun, 07 Feb 2021 23:22:42 +0000

What is the userv model?

A major vulnerability in Sudo

marcH — Sat, 06 Feb 2021 00:45:04 +0000

No, in this particular context "comes for free" (maybe not the best expression here) means: "it's memory safe or it does not compile".

It's like a static C analyser but built in, not optional and with barely any false positive ever.

A major vulnerability in Sudo

ssmith32 — Fri, 05 Feb 2021 22:35:55 +0000

Comes for free just means it's more likely to be used, not necessarily it will always be used.

But it's all about risk management anyways, so might as well try to skew the odds in your favor.

A major vulnerability in Sudo

nix — Fri, 05 Feb 2021 18:50:33 +0000

Please. It's *cat* that plays mp3s. (Literally true, on my system. I can also play mp3s with lpd.)

A major vulnerability in Sudo

mathstuf — Thu, 04 Feb 2021 17:58:47 +0000

A lot of that comes at the expense of overhead in cross-library calls/member lookups. Either you force a function call, have a lookup table of member offsets, or have some weird inline code copying semantics. I believe Rust likely does not want to add such things to the language (at least at this time; it is likely at least in the direction of any likely solution that may appear eventually).

A major vulnerability in Sudo

smoogen — Thu, 04 Feb 2021 11:11:31 +0000

Sadly parsing sh is the first thing everyone seems to do with a suid program if it didn't before. It is like a moth to a flame.. ooooh I need this thing to do a shell script, it can't be that hard.

When I was younger, I was naive to believe that most of the core programs could be easily replaced with simpler programs.. and you could just keep to doing that one thing. Then you find that most of the people who espouse the Unix philosophy also think that if you made ooone little change to /bin/rm it would be much better for their usecase.. and 8 billion users later, rm now is setuid, sends email and plays mp3s.

A major vulnerability in Sudo

Jonno — Thu, 04 Feb 2021 09:44:41 +0000

> It seems at the moment the price you pay for memory safety is the inability to have shared libraries (at least with "safe" interfaces).

Rust does allow you to use the full rust ABI in shared libraries, but the Rust ABI is still unstable between compiler releases. If you want ABI stability you are restricted to the C ABI, which is inherently unsafe. Creating a stable Rust ABI is on the roadmap, but not for anytime soon....

A major vulnerability in Sudo

Cyberax — Thu, 04 Feb 2021 09:25:37 +0000

Ask sudo developers. The answer is likely "it was inconvenient at that moment".

A major vulnerability in Sudo

NYKevin — Thu, 04 Feb 2021 08:32:35 +0000

Right, but to do the escaping, you had to make a copy to begin with (because the string gets longer). So why not keep the original? How on Earth does it make sense to un-escape the copy?

A major vulnerability in Sudo

marcH — Thu, 04 Feb 2021 06:51:39 +0000

https://en.wikipedia.org/wiki/Null-terminated_string: the other billion dollar mistake?

> It’s really not a good strings language. It’s just not.

The language of manual memory management and corruption.

A major vulnerability in Sudo

Cyberax — Thu, 04 Feb 2021 06:47:27 +0000

Nothing at all stops you from making shared libraries with safe interfaces. Swift does that just fine, as an example.

A major vulnerability in Sudo

epa — Thu, 04 Feb 2021 06:39:28 +0000

Yes, if you are going to run sh -c "command", where command hasn't been passed as a single argument but somehow as separate args, then you need to do fancy escaping so that the command line is built correctly. The answer is not to offer this functionality at all, it's too error-prone. As others noted, this is only the less used --shell and --login modes. Better to require the user to escalate using a mechanism that's as simple as possible, and then if the user wants to run sh -c that's their decision, once the authentication has been safely done. sudo's own manual page gives an example of this use:
$ sudo sh -c "cd /home ; du -s * | sort -rn > USAGE"

A major vulnerability in Sudo

plugwash — Thu, 04 Feb 2021 02:34:13 +0000

It seems at the moment the price you pay for memory safety is the inability to have shared libraries (at least with "safe" interfaces). So, in addition to the memory/cache costs of duplicate libraries, if one of the libraries does turn out to have a security flaw you have a much bigger job fixing it.

A major vulnerability in Sudo

marcH — Wed, 03 Feb 2021 22:04:39 +0000

> That makes review of these far easier and more targeted for these kinds of problems. With C and C++, the entire codebase is under such a block

Exactly, "git grep unsafe" and win.

This is once again not a yes/no question, it's all about *how much* time is spent in code review and other quality assessments = what every manager say they want but never allocate proper resources for.

And to be clear, when I wrote "it comes for free" I meant _memory safety_ comes for (almost) free. Of course switching to a different programming language is everything but free and even practically impossible for some projects.

A major vulnerability in Sudo

Cyberax — Wed, 03 Feb 2021 21:24:41 +0000

Unescaping always decreases the string length, so you can keep the previous buffer and just slap a NUL character at the new end.

A major vulnerability in Sudo

NYKevin — Wed, 03 Feb 2021 21:01:35 +0000

The part I just realized today: sudo has to make a copy anyway, because backslash escaping modifies the length of the string. So now I *really* don't get this bug.

A major vulnerability in Sudo

mathstuf — Wed, 03 Feb 2021 20:12:54 +0000

Note that in Rust, many of the "core" crates have comments explaining *why* each and every `unsafe` block is necessary. That makes review of these far easier and more targeted for these kinds of problems. With C and C++, the entire codebase is under such a block and "no one" comments their safety guarantees at that granularity.

A major vulnerability in Sudo

smoogen — Wed, 03 Feb 2021 19:02:21 +0000

I would beware of 'it comes for free'. I have seen that argument used over and over for 30 years.. and inevitably found to be flawed... usually because someplace in execution the program has to go into whatever that language 'unsafe' mode is. Then all the security bets are off HOWEVER the programmer and reviewers usually just say 'well it was in XYZ language so it can't be broken'. And then we have reliably 4 years after the language has gotten through the hype cycle of all the security problems that no one fuzzed for.

A major vulnerability in Sudo

smoogen — Wed, 03 Feb 2021 18:58:43 +0000

I think sudo did that a looong time ago, and it was one of the original 'how to break out of sudo'. So it had to gain parsing to try and deal with a lot of corner cases where passing things willy nilly to another shell or program caused worse issues.

A major vulnerability in Sudo

dd9jn — Wed, 03 Feb 2021 16:37:44 +0000

It is a pity that the simple userv model is so less known. It does all the things right which you could get wrong with privilege elevation. sudo instead tries to be everyone's best friend and makes things often too easy.

A major vulnerability in Sudo

epa — Wed, 03 Feb 2021 16:10:23 +0000

It may be better to keep tricky code (like escaping things for sh -c) in one place, but a suid-root binary is not the place I'd pick.

A major vulnerability in Sudo

ibukanov — Wed, 03 Feb 2021 16:00:22 +0000

If the log should allow reconstruction of the whole command as-is with all special characters and spaces, sudo could do it when writing the log appending escapes to the log file as necessary. This way the input can be const pointers and in the worst case the code will read pass the allocated buffer, not write.

A major vulnerability in Sudo

ibukanov — Wed, 03 Feb 2021 15:55:21 +0000

Typical usage that requires a shell is not to call a shell built-in, but to do a redirections with <, > when the whole command should be passed as single argument. And even if I need to call a built-in with arguments, I can always do it like 'builtin $0 $@' arg1 arg2 ...

What sudo does is way too smart given the danger of bugs in it.

A major vulnerability in Sudo

Paf — Wed, 03 Feb 2021 14:39:45 +0000

As a career long C developer...

This, a thousand times this. It’s really not a good strings language. It’s just not.

A major vulnerability in Sudo

Jonno — Wed, 03 Feb 2021 14:25:11 +0000

> Why does sudo need any argument escaping at all? Why cannot it pass the arguments to /bin/sh -c directly?

Because sudo gets the command to run and each of its arguments as separate entries in argv[], but sh -c expects the command and all its arguments as a single entry in its argv[], which it will interpret...

A major vulnerability in Sudo

tzafrir — Wed, 03 Feb 2021 13:52:04 +0000

Because it logs the command. And there are some cool tricks you can do with various characters that get into the logs.

A major vulnerability in Sudo

roc — Wed, 03 Feb 2021 12:24:33 +0000

What languages make it hard to iterate through the characters of a string? I can't think of a single one.

C lacks solid string and buffer primitives regardless of how you lampshade it. fmemopen and open_memstream are massive overkill for string manipulation ... they both implement locking by default! C just doesn't have an ergonomic way to track bounded string/array slices or growable buffers. All the library solutions are excessively verbose, so developers are constantly tempted to write hand-rolled code with raw pointers and manual buffer size calculations, and they regularly get it wrong with disastrous consequences.

A major vulnerability in Sudo

ibukanov — Wed, 03 Feb 2021 12:17:41 +0000

Why does sudo need any argument escaping at all? Why cannot it pass the arguments to /bin/sh -c directly?

A major vulnerability in Sudo

tschoerbi — Wed, 03 Feb 2021 12:10:56 +0000

It might be time to replace sudo. Almost always, you just want to be able to grant one group root access for which sudo's complexity isn't needed.

On my systems, I have had permission to sudo restricted for years. I only every need group sudo to be able to execute sudo:

dpkg-statoverride --force-statoverride-add --update --add root sudo 4750 /usr/bin/sudo

A major vulnerability in Sudo

Jonno — Wed, 03 Feb 2021 11:08:36 +0000

> Sudo has no business running something fancy like sh -c "command"; it should do the stupidest thing possible, which is to take its literal arguments from the argv array and pass them directly to exec(). Sadly, it may be too late to change.

That is indeed what sudo does by default. It is only when you pass both "-s" / "--shell" (run a shell) or "-i" / "--login" (run a login shell) as well as specifying a command to execute that sudo starts going haywire, by passing the command to the shell using the "-c" argument of the shell in question.

Having sudo execute the specified command through a shell running as the target user is useful for running shell builtins, doing shell redirections, etc; so the alternative to sudo doing the escaping is the caller doing them and then calling `sudo -- /bin/sh -c "..."`. It is _usually_ better to keep tricky stuff like this in one place rather than a hundred, but that presupposes that someone actually checks that it does so correctly...

A major vulnerability in Sudo

epa — Wed, 03 Feb 2021 10:44:24 +0000

Personally, as soon as I hear the words "escape it with backslashes" I start to worry. That sort of thing is a cosy nest for bugs, even in a memory-safe language. Sudo has no business running something fancy like sh -c "command"; it should do the stupidest thing possible, which is to take its literal arguments from the argv array and pass them directly to exec(). Sadly, it may be too late to change.

A major vulnerability in Sudo

marcH — Wed, 03 Feb 2021 08:47:45 +0000

> For a bug of this sort to persist for this long in a tool of this nature would seem to indicate that we are not really scrutinizing our code as well as we should be. Or testing and fuzzing enough either.

"We" generally don't find this fun and "we" rarely get paid for any of that either. At least not on the defence side.

People who actually care about memory safety are just moving to safe languages - why would they bother about all this tedious and error-prone review and fuzzing work when that part comes "for free" in newer languages? Every minute of work spent on memory safety is a minute not spent on something more productive - including of course other, higher level security issues.

A major vulnerability in Sudo

wahern — Wed, 03 Feb 2021 07:20:35 +0000

Also, as has recently been made clear to me today while discussing severity ratings during a security review, unless people *see* things break they don't take it seriously. In fact, because convenient, copy+pastable root shell PoCs weren't immediately available last week, Tenable rated the CVE *low* severity, which is just patently ridiculous to me. If heap memory corruption in a tool like sudo is proven with a one-liner, it's absolutely negligent to harbor any doubts about present exploitation potential, regardless of where one falls on the spectrum of risk averseness.

I'm normally quite cynical when it comes to Linux security--my background assumption is that if someone has shell access on a Linux host, they have root access. But holding that opinion is no excuse for neglecting a diligent and accurate assessment of relative threat potential.