DEV Community

Laurent Laborde
Laurent Laborde

Posted on

Sudo (CVE-2021-3156) - I went looking for a CVE exploit and found a different bug instead, which got patched.

As i'm migrating from old Diary, i've decided to repost just a few selected post that i consider worth keeping. Not for the quality of the post, but for the content. I try to reformat it to dev.to standard anyway. (Even though it appears that "dev.to" standard is AI Slop). This post was written @ 16 January 2024.

Exploring CVE-2021-3156 @ home

Sudo before 1.9.5p2 contains an off-by-one error that can result in a heap-based buffer overflow, which allows privilege escalation to root via "sudoedit -s" and a command-line argument that ends with a single backslash character.

This is what I understood :

  • You can use multiple line in argument by escaping with \
  • Sudo ignore the character following \
  • what if \ is the last character ? it ignores \0 (NULL) and read stuff it shouldn't read because the null terminator is ignored.

Let's find out.

  • The commit fixing the bug is here
  • and here
  • and ... here ?
  • This was also submitted by Qualys the same day, let's assume it's part of it

Well... it's not as simple as "we forgot that having a backlash as last character could happen". No, no, no. It's an error with flags and stuff, because sudo DO check for this. (I'll have to check for sure but I assume it does)

Also, sudo and sudoedit are the same binary. sudoedit is just a symlink to sudo, and sudo is checking its own name to set some flags here and there in order to behave as "sudo" or as "sudoedit".
Well, that's what I understood anyway. I'll check, of course.

My immediate thought was : what if it's called neither "sudo" nor "sudoedit" ? huh ? huh ? aren't I smart ?
Well... I'm not the smartest one.

This is the patch published 3 days after the CVE fix : The program name may now only be "sudo" or "sudoedit".

Anyway... code stuff !

The old code :

    /* Pass progname to plugin so it can call initprogname() */
    progname = getprogname();
    sudo_settings[ARG_PROGNAME].value = progname;

    /* First, check to see if we were invoked as "sudoedit". */
    proglen = strlen(progname);
    if (proglen > 4 && strcmp(progname + proglen - 4, "edit") == 0) {
    progname = "sudoedit";
    mode = MODE_EDIT;
    sudo_settings[ARG_SUDOEDIT].value = "true";
    }
Enter fullscreen mode Exit fullscreen mode

This is honestly a weird way to check if it's invoked as "sudoedit". It could technically be "anythinglongerthan4edit".
Why ? dunno ! Probably very legacy.

The new check is as easy as :

    /* The plugin API includes the program name (either sudo or sudoedit). */
    progname = getprogname();
    sudo_settings[ARG_PROGNAME].value = progname;

    /* First, check to see if we were invoked as "sudoedit". */
    if (strcmp(progname, "sudoedit") == 0) {
    mode = MODE_EDIT;
    sudo_settings[ARG_SUDOEDIT].value = "true";
    valid_flags = EDIT_VALID_FLAGS;
    }
Enter fullscreen mode Exit fullscreen mode

While we're here we can clearly see one bigass fix :

valid_flags = EDIT_VALID_FLAGS;

Well... I know it is because YouTube told me it was about flags. ;)

The new code is still weird, imho.
It says : The plugin API includes the program name (either sudo or sudoedit).
But this is not what the code is doing. it checks if it's "sudoedit" or "anything else".

Anyway... I'm on my windows, I have WSL and ubuntu installed.

sudoedit --version
sudoedit: Only one of the -e, -h, -i, -K, -l, -s, -v or -V options may be specified
usage: sudoedit [-AknS] [-r role] [-t type] [-C num] [-g group] [-h host] [-p prompt] [-T timeout] [-u user] file ...
Enter fullscreen mode Exit fullscreen mode

huh ? okay then ...

sudoedit -V
sudoedit: Only one of the -e, -h, -i, -K, -l, -s, -v or -V options may be specified
usage: sudoedit [-AknS] [-r role] [-t type] [-C num] [-g group] [-h host] [-p prompt] [-T timeout] [-u user] file ...
Enter fullscreen mode Exit fullscreen mode

C'mooon... really ?

how about this ?

sudo -V
Sudo version 1.8.31
Sudoers policy plugin version 1.8.31
Sudoers file grammar version 46
Sudoers I/O plugin version 1.8.31
Enter fullscreen mode Exit fullscreen mode

I tried on my mac M1. "sudo -V" works. and sudoedit is "command not found".

I'm starting up a linux cloud instance.

sudo -V
Sudo version 1.8.21p2
+ a hundred lines of stuff
Enter fullscreen mode Exit fullscreen mode
sudoedit -V
sudoedit: Only one of the -e, -h, -i, -K, -l, -s, -v or -V options may be specified
usage: sudoedit [-AknS] [-r role] [-t type] [-C num] [-g group] [-h host] [-p prompt] [-T timeout] [-u user] file ...
Enter fullscreen mode Exit fullscreen mode

nice ! I found a boring bug ! :o)

  • it should not happen. the usage() clearly say that -v and -V are valid.
  • There are some other potential issues still happening with sudoedit. could it lead to a security issue ? dunno.

So let's check parse_args.c, again.

    /* Is someone trying something funny? */
    if (argc <= 0)
    usage();
Enter fullscreen mode Exit fullscreen mode

No, I'm not. I'm checking all calls to usage() from parse_args.

        case 'V':
            if (mode && mode != MODE_VERSION)
            usage_excl();
            mode = MODE_VERSION;
            valid_flags = 0;
            break;
        default:
            usage();
Enter fullscreen mode Exit fullscreen mode

mmm ?
Let's try something.

# sudo -z
sudo: invalid option -- 'z'
usage: sudo -h | -K | -k | -V
usage: sudo -v [-AknS] [-g group] [-h host] [-p prompt] [-u user]
usage: sudo -l [-AknS] [-g group] [-h host] [-p prompt] [-U user] [-u user] [command]
usage: sudo [-AbEHknPS] [-r role] [-t type] [-C num] [-g group] [-h host] [-p prompt] [-T timeout] [-u user] [VAR=value] [-i|-s] [<command>]
usage: sudo -e [-AknS] [-r role] [-t type] [-C num] [-g group] [-h host] [-p prompt] [-T timeout] [-u user] file ...
Enter fullscreen mode Exit fullscreen mode
# sudoedit -z
sudoedit: invalid option -- 'z'
usage: sudoedit [-AknS] [-r role] [-t type] [-C num] [-g group] [-h host] [-p prompt] [-T timeout] [-u user] file ...
Enter fullscreen mode Exit fullscreen mode

ho so there are two different usage() for sudo and sudoedit.

/*
 * Tell which options are mutually exclusive and exit.
 */
static void
usage_excl(void)
{
    debug_decl(usage_excl, SUDO_DEBUG_ARGS);

    sudo_warnx("%s",
    U_("Only one of the -e, -h, -i, -K, -l, -s, -v or -V options may be specified"));
    usage();
}
Enter fullscreen mode Exit fullscreen mode

Okay ... actually so the warning I get isn't from usage() but from usage_excl()

Let's check the 1.8.21p2 source code just in case

The source I got from apt source shows :

case 'V':
    if (mode && mode != MODE_VERSION)
        usage_excl(1);
    mode = MODE_VERSION;
    valid_flags = 0;
    break;
default:
    usage(1);
Enter fullscreen mode Exit fullscreen mode
/*
 * Tell which options are mutually exclusive and exit.
 */
static void
usage_excl(int fatal)
{
    debug_decl(usage_excl, SUDO_DEBUG_ARGS)

    sudo_warnx(U_("Only one of the -e, -h, -i, -K, -l, -s, -v or -V options may be specified"));
    usage(fatal);
}
Enter fullscreen mode Exit fullscreen mode

It's pretty much the same I guess ?

The source also show that the main bug that issued the CVE is fixed :

    /* First, check to see if we were invoked as "sudoedit". */
    proglen = strlen(progname);
    if (proglen > 4 && strcmp(progname + proglen - 4, "edit") == 0) {
        progname = "sudoedit";
        mode = MODE_EDIT;
        sudo_settings[ARG_SUDOEDIT].value = "true";
        valid_flags = EDIT_VALID_FLAGS;
    }
Enter fullscreen mode Exit fullscreen mode

The flag is here. mode is set and different than MODE_VERSION so it indeed show usage_excl(). But why ?

After some time thinking about it, it's obvious :

        case 'V':
            if (mode && mode != MODE_VERSION)
            usage_excl();
            mode = MODE_VERSION;
            valid_flags = 0;
            break;
Enter fullscreen mode Exit fullscreen mode
  • But why does it call usage_excl() here ?
  • why is mode set but it's clearly not MODE_VERSION ?
  • Because sudoedit set mode = MODE_EDIT;
  • I'll submit a bug report and see how it goes.
  • First day, first bug. yay \o/

Bug report


  • That was one big messy daily report. I wanted to explore a CVE and found a (minor) bug instead.
  • That's even better, isn't it ?
  • Or not, my bug was (partially :( )) fixed and closed already.
  • and I found another problem (pretty much the same bug actually) when calling sudoedit -h
  • I think I understand why there is no sudoedit on mac. it shouldn't exist in the first place, imho.
  • When you read this diary you clearly see that I wrote stuff that were clearly incorrect. I'm not removing it.
  • it accurately describes my thought process and I'm not always right on first try.

2021/03/31

  • Dear diary, it's morning already. I'm not working today. More CVE ?
  • It turn out that Macos also have (had) the sudoedit problem.
  • sudoedit does not exist by default on mac but the code is still here anyway
  • just create a sudoedit symlink

    ln -s /usr/bin/sudo sudoedit

  • it does not require any privilege to be created

  • the sudoedit code is still in the sudo binary

% ln -s /usr/bin/sudo sudoedit                   
% ./sudoedit -V
sudoedit: Only one of the -e, -h, -i, -K, -l, -s, -v or -V options may be specified
usage: sudoedit [-AknS] [-C num] [-D directory] [-g group] [-h host] [-p prompt] [-R directory] [-T timeout] [-u user] file ...
% rm sudoedit
Enter fullscreen mode Exit fullscreen mode
  • My bugreport has been closed with a patch that check if progname is "sudoedit"
  • They also added a separate getopt config for sudoedit
case 'V':
    if (mode && mode != MODE_VERSION) {
    if (strcmp(progname, "sudoedit") != 0)
        usage_excl();
    }
Enter fullscreen mode Exit fullscreen mode
static const char sudo_short_opts[] = "+Aa:BbC:c:D:Eeg:Hh::iKklnPp:R:r:SsT:t:U:u:Vv";
static const char edit_short_opts[] = "+Aa:BC:c:D:g:h::knp:R:r:ST:t:u:V";
+ more stuff to handle the new getopt config
Enter fullscreen mode Exit fullscreen mode

It's not my code, so I can't say that my code has been added to billions of devices. But it's because of my report, and this Diary, that the code has been patched. It still feels good :)

Top comments (0)