A major vulnerability in Sudo

A longstanding hole in the Sudo privilege-delegation tool that was discovered in late January is a potent local vulnerability. Exploiting it allows local users to run code of their choosing as root by way of a bog-standard heap-buffer overflow. It seems like the kind of bug that might have been found earlier via code inspection or fuzzing, but it has remained in this security-sensitive utility since it was introduced in 2011.

Qualys reported the bug on January 26; it has been in Sudo from version 1.8.2, released in August 2011, up through 1.9.5p1, which was released on January 11. At the same time as the announcement, Sudo released version 1.9.5p2 to fix the problems. The bug has been assigned CVE-2021-3156, which Qualys has dubbed "Baron Samedit". That name combines Baron Samedi, the name of the vodou loa of the dead, with sudoedit, which is integral to the exploit.

Unlike last year's Sudo vulnerability, which exploited an uncommon, non-default configuration, this time around the problem is more widespread. Any systems with untrusted users will want to upgrade Sudo to avoid the problem. The major Linux distributions have already issued updates at this point.

One fairly straightforward test to see if a system is vulnerable is shown in the report:

    $ sudoedit -s '\' `perl -e 'print "A" x 65536'` 
    malloc(): corrupted top size 
    Aborted (core dumped)

Systems that have the problem will see the "corrupted top size" warning message. Another, perhaps less worrisome test, sans scary messages, is shown in the Sudo project advisory:

    $ sudoedit -s /
    sudoedit: /: not a regular file     # or it might prompt for a password

In both cases, systems that are not vulnerable will simply give a usage string. As we will see, attackers can control what gets written beyond the end of a heap buffer, which effectively allows them to make the program do their bidding. That means attackers can subvert sudoedit, which is simply a symbolic link to the setuid-root sudo binary. Game over, as they say.

The "-s" option is important to the flaw, but it is also not really meant for sudoedit at all; it specifies that the user's shell should be used, but sudoedit invokes an editor. The "-s" (or "--shell") option is valid and does make sense for the underlying sudo command, however. Part of the fix for CVE-2021-3156 is to restrict the valid options for sudoedit, which is why the patched versions give a usage string instead of a crash or other error.

Defeating the escape code

The buffer overflow itself also needs to be addressed, even though the option fix leaves no known path to get to that code. The basic problem is that a command-line argument that ends with a backslash can break the code that escapes meta-characters during argument parsing, leading to writing past the end of a heap-based buffer.

When sudo is executing in shell mode (due to -s or the related -i), it collects up all of the command-line arguments into a single buffer and escapes any meta-characters found with a backslash. It creates a new version of the argv array that consists of the shell to be executed, "-c" as the command argument to the shell, followed by this new buffer. That new argv will then be used when the shell is executed.

Later in the processing, Sudo processes the new argv and, if necessary, un-escapes the meta-characters into another buffer in order to match them in the sudoers file as well as for logging purposes. But that un-escaping process can go awry if any command-line argument ends with a single backslash. Under that condition, the un-escaping code copies the character after the backslash, even if it is the NUL at the end of the string. It merrily keeps on copying until it hits an un-escaped NUL, so it copies into a heap buffer beyond its end, the size of which was calculated based on the first NUL.

In theory, every buffer that gets un-escaped has already been escaped internally so every backslash has been escaped with another. If true, that would mean this condition could not occur, but, of course, it turns out not to be true in (at least) one case. The test used to decide whether to force the escaping is subtly different than the test to decide whether to un-escape. By using sudoedit with the -s option, the "proper" path could be tickled, so that the options were not escaped, but were later un-escaped, which led to the buffer overflow. That was fixed with a separate patch addressing both the tests and the broken logic when the buffer ends with a single backslash.

As described by the report, this buffer overflow is ideal from the attacker's perspective because they can control everything about the contents and size of the overflow. The last command-line argument to sudo is conveniently followed by the environment variables, so the attacker can precisely arrange the contents and even include NUL bytes since ending arguments or environment variables with a single backslash results in a NUL. The report gives an example of how to write attacker-controlled values to the data structure used by malloc():

env -i 'AA=a\' 'B=b\' 'C=c\' 'D=d\' 'E=e\' 'F=f' sudoedit -s '1234567890123456789012\'
------------------------------------------------------------------------

--|--------+--------+--------+--------|--------+--------+--------+--------+--
  |        |        |12345678|90123456|789012.A|A=a.B=b.|C=c.D=d.|E=e.F=f.|
--|--------+--------+--------+--------|--------+--------+--------+--------+--
              size  <---- user_args buffer ---->  size      fd       bk

There is even more information about exploiting the flaw in the "Exploitation" section of the report. It makes for some fascinating reading for those who are curious about how these kinds of exploits can work.

In the end, though, this is caused by a typical C programming error leading to a complete compromise of a setuid-root program—but only for local users. It once again highlights the dangers of using C for these kinds of tools, but it also points to a certain amount of complacency within our community. 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.

As Hanno Böck noted, though, sudo likely has far more complexity than a tool of this sort should have; there are alternatives, but those have potential downsides as well. Wholesale replacement in a safer language (and perhaps with many fewer features) has its attractions, but someone has to do that work too; then there is a new code base, in a possibly less-familiar language, that needs a lot of scrutiny as well. As always, there are no silver bullets.