Anatomy of a security hole - the break that broke sudo

Filed Under: Featured, Vulnerability

A recently-patched bug in the widely-used sudo command makes for interesting reading.

The patch reminds us of some important points in security-related programming.

The UNIX sudo command is usually translated to mean "do as superuser" - in other words, it lets you run administrative commands one at a time, without logging in as the superuser (root, in UNIX parlance).

More accurately, sudo stands for "substitute user identity and do", so that you can use it to take on an identity other than root, such as a backup user or a database administrator.

Clearly, sudo is the sort of program in which a vulnerability will almost certainly lead to an escalation-of-privilege exploit. Since sudo's main purpose is to let you enjoy root-like powers in a controlled way, anyone who can sneak past sudo's permission checks can turn a minor intrusion into an effectively unlimited one.

So why use a potentially risky command like sudo at all? The answer is that sudo can improve security significantly:

* You don't need to tell all your administrative users the root password. Sudo lets them authenticate individually, and keeps a log of each individual's administrative activity.

* You don't need to work at a root prompt. This avoids running every command with supreme power. You take on root privileges only when they are strictly necessary.

* You can authorise users to run some, but not all, administrative commands. You can let them start a backup, for example, but not add new users to the system.

In fact, some operating system distributions - the Ubuntu family, for example, and OS X - discourage root login altogether, and use sudo for all administrative tasks.

This isn't perfect, so caution is still required.

Sudo lets you activate a root prompt if you want - for example, with the commands sudo su - or sudo -s.

And many users, confronted with a Permission denied error, fall into the bad habit of repeating their previous command with sudo in front. (If you have developed this habit, get rid of it. Use sudo because you ought to, not merely because it's convenient.)

Anyway, sudo was recently found to have a single-line bug which affected its user authentication in rather specific circumstances.

Sudo is controlled by a configuration file called sudoers, or by similar configuration data managed by LDAP directory services.

Amongst other things, this file lets you regulate the power of users by the network they're on. When sudo sets out to verify a user's network mask - for example, 203.0.113.0/24 - it enters a C function called addr_matches_if_netmask().

The authorisation part of this function is split into two parts. One part checks IPv4 addresses; the other checks IPv6 addresses.

The function chooses which path to take using a switch statement. Switches are one of C's decision-making instructions, and they look something like this:

   switch (dayofweek) {
      case SUNDAY: 
         do_sunday_stuff();
         break;
      case SATURDAY:
         do_saturday_stuff();
         break;
      default:
         do_weekday_stuff();
         break;
   }

Notice the break statements. These tell C, "That's everything for this code path through this switch, thanks. Exit the switch right now."

However, you can legally omit any break, if you want processing to fall through to the next case in the switch:

   switch (dayofweek) {
      case SUNDAY: 
         do_special_sunday_stuff();
         break;
      case SATURDAY:
         do_saturday_stuff();
         /* Break omitted - deliberate fall-through */
         /* to do non-Sunday stuff on Saturday too. */
      default:
         do_nonsunday_stuff();
         break;
   }

And that's what happened in sudo. The break statement between the IPv4 checking and the IPv6 checking was left out. So if the IPv4 checks failed, the IPv6 checks - inappropriate in the circumstances - were tried instead. With nothing to match against nothing, the checks succeeded, even though they had already failed in the IPv4 code above.

Result: users whose sudo access was regulated by network masks would be authenticated when they shouldn't be. So an attacker could masquerade as an authorised internal user, even from outside the network.

And, as promised at the outset, this reminds us of some important points in security-related programming:

* Bugs can be arcane enough to escape notice for years. This can happen even in security software, and even if the source code is freely available.

* Bugs can be caused even (or especially!) by minuscule errors.

* Bugs are worth patching.

* Watch your breaks.

By the way, always put a break after the last case in a switch. It might not be necessary now, but it ensures that coders who add cases later don't forget the now-necessary breaks above their new code.

Technically, if you don't use IPv4 network masks in your sudo configuration, you aren't affected by this bug. But since almost every Linux or UNIX has sudo installed, it's worth upgrading.


-

The XKCD 'Sandwich' is comic #149 from http://xkcd.com/ - "a webcomic of romance,
sarcasm, math, and language.
"

Note to Windows users: your equivalent of sudo is runas. Try runas /? at a command prompt to find out more."

, , , , , , , , ,

You might like

24 Responses to Anatomy of a security hole - the break that broke sudo

  1. asfonseca · 800 days ago

    In the list of reasons to use sudo you forgot to mention one that is really important: using sudo activities are logged in the system log files.

    • Paul Ducklin · 800 days ago

      Errr, I wrote in my first point: "Sudo lets [multiple admin users] authenticate individually, and keeps a log of each individual's administrative activity."

    • »sudo bash«
      There goes your logging ...

      Oh, I am only allowed to edit files?
      »sudo vi /tmp/bla«
      »:!bash«
      There goes your logging ...

      • Paul Ducklin · 800 days ago

        Indeed, as we mentioned above if you do sudo su - or sudo -s.

        However, logging would at least show administrative activity _by user X_. Which is more than you get if you just see "user root logged in and did stuff," with no disambiguation of "which admin".

  2. jmb · 800 days ago

    "sudo -s", surely?

    • Paul Ducklin · 800 days ago

      Surely.

      I've updated the article (and the image) to clarify that there are several ways to get a root prompt. I now mention sudo -s and sudo su -, and sudo /bin/sh and sudo login -f root spring to mind, too.

  3. Roger owens · 800 days ago

    I used to lecture the software engineers (that I managed b4 I retired) on this very point. Always use the break or use a compound if. If statements make for better code when you only have two conditions, but don't forget the else statements. If you want to use switches cause they're easier to see the logic, always perform peer code reviews. And, furthermore, always perform exhaustive tests!

    • Paul Ducklin · 800 days ago

      I too would have preferred

      if (it_is_IPv4) {
      /* do ipv4 stuff */
      } else if (it_is_IPv6) {
      /* do ipv6 stuff */
      } else {
      /* it's an error situation */
      }

      with a switch introduced only if there are more than two legal alternatives. Switches can often let the compiler generate more efficient code, but only for a decently-sized set of alternatives.

  4. Xorinzor · 800 days ago

    I dont get it, in PHP calling a switch statement but forgetting the break will result in a PHP error, not so weird as you cant call "case" in a "case" function without a switch surrounding it, how could this happen here?

    • Paul Ducklin · 800 days ago

      Sudo is written in C, not in PHP.

      As mentioned in the article, "[in C] you can legally omit any break, if you want processing to fall through to the next [case] in the switch."

      (See the example I gave - where you want the Saturday case to perform the default case's code, too.)

    • Jorrit Kronjee · 800 days ago

      This is not true. PHP allows omitting breaks in a switch statement.

      Here's a direct excerpt from their manual: "PHP continues to execute the statements until the end of the switch block, or the first time it sees a break statement. If you don't write a break statement at the end of a case's statement list, PHP will go on executing the statements of the following case."

  5. dav2 · 799 days ago

    "But cince almost every Linux or UNIX has sudo installed, it's worth upgrading."

    ... since

  6. Mark Fisher · 799 days ago

    Unit tests should show this sort of thing pronto - I can't believe they have not been implemented for such a ubiquitous command. Typical!

    • Foo · 797 days ago

      If it's typical, you *should* be able to believe they've not been implemented.

  7. chipbuster · 799 days ago

    Aha! Just a few days ago, I was complaining that Ubuntu released an update that "changed sudo behavior" but didn't provide any easily accessible details. I suppose this is what that was about?

    Also, I've heard from several sources that simply doing a "sudo !!" is bad practice, but I haven't seen any examples. Could you show where a sudo !! could be avoided and how to do it? Usually what happens to me is I try to apt-get upgrade and have to put a sudo in front; as far as I know there's no other way to do that.

    • krill · 797 days ago

      You should explicitly retype the command, or recall it from your command history and sudo it, I think. Running commands with elevated privileges should be explicit, not convenient.

      • Paul Ducklin · 797 days ago

        Agreed.

        I use my command history so I can review what I just typed if I think I needed "sudo".

        Did the command fail because I made a mistake? Did the command fail because I forgot to ask it _not_ to try to perform actions which would require admin privileges? Or was it really a command I intended to, and ought to, run as root?

        If you're really serious about security, you probably want to use apt-get to --download-only running as yourself and only do the actual install (from your local repository) running as root :-)

  8. Christy Nicol · 797 days ago

    If the code sample you show is accurate then the AF_INET6 block isn't being executed unless AF_INET4 and AF_INET6 happen to have the same value. So the bug isn't quite what you described. Although the missing break is the cause, what is happening is a bit more subtle. (Unless I'm missing something. Haven't done C in a while.)

    • Christy Nicol · 797 days ago

      Oh wow I suppressed how non-intuitive the C switch statement is. What a great language feature.

  9. Cogito · 797 days ago

    Ubuntu is part of the Debian family of distributions. Does that make members of the Ubuntu family part of the Debian family too?

    • Paul Ducklin · 797 days ago

      Do you mean distros like Lubuntu and Kubuntu? (Officially-recognised Ubuntu variants with different window managers.)

      If so, I guess so.

      I think the best way of describing the relationship, lest Ubuntu be thought a superset of Debian (i.e. it has all Debian's benefits, and more) is to say, "Ubuntu variants are derived from Debian. But Ubuntu != Debian."

      Try this graphical family tree of Linux distroland...
      http://upload.wikimedia.org/wikipedia/commons/8/8...

      If the compilers of this are to be believed, most Linuxes are part of just three tribes: Debian, Red Hat and Slackware. (And Slackware, of course, comes from the very first ever distro, Softlanding Linux System, or SLS.)

  10. Elliott · 796 days ago

    A possibly unexpected side-effect of this column... I usually understand xkcd but that one had me stumped. Thanks for the explanation.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

About the author

Paul Ducklin is a passionate security proselytiser. (That's like an evangelist, but more so!) He lives and breathes computer security, and would be happy for you to do so, too. Paul won the inaugural AusCERT Director's Award for Individual Excellence in Computer Security in 2009. Follow him on Twitter: @duckblog