
Potential bypass of Runas user restrictions in sudo - Findus23
https://www.sudo.ws/alerts/minus_1_uid.html
======
gambler
Is this the official sudo repository?

[https://www.sudo.ws/repos/sudo/file/f75f786eddd5](https://www.sudo.ws/repos/sudo/file/f75f786eddd5)

It has more than 10 thousand commits, ~600 files, and close to 11MB of C code.
Also, the code seems to have no unit tests, the main file is 1.4K lines long,
has quintuple-nested conditionals and liberally uses goto statements.

Am I missing something here?

~~~
jblow
Some of the points you raise are matters of style (I have files that are 10k
lines long and think they are better that way; also I don't think unit tests
are as useful as claimed), but yeah, when you have 600 files and 11MB of code
to do something that is supposed to be simple and also is security-critical
... you have a big problem.

~~~
gambler
What a coincidence. I watched your video on Preventing the Collapse of
Civilization just yesterday. (Mostly agree with things said there, with the
exception of the part about Smalltalk. Ironically, the video was recommended
to me in a Smalltalk chat channel.) Seems relevant.

 _> Some of the points you raise are matters of style (I have files that are
10k lines long and think they are better that way_

It's true that line counts are a matter of style. However, some coding styles
tax cognitive bandwidth and immediate memory capacity more than others. This
causes people to skim over specifics and miss bugs. Even if you are
comfortable with a 10K LOC file you wrote, whomever reviews your code will
probably be overwhelmed until they fully comprehend its structure. Grouping
related things into smaller files is a way to focus their attention and
communicate intended relations between code units.

 _> also I don't think unit tests are as useful as claimed_

In general, I agree. Not a fan of TDD zealotry. But in this particular case
_some_ unit tests would be beneficial. There is a lot of stuff going in some
methods.

They do seem to run PVS-Studio static analysis, which is somewhat reassuring,
but I don't think that's enough in code that is so important, complex and
widely used.

~~~
jblow
It should be very obvious once you think about it that breaking code into more
files, and procedures into subprocedures, is pushing complexity around rather
than simplifying ... and it’s pushing the complexity from somewhere visible,
into an invisible structure that the viewer has to then reconstruct in his
mind. I think in a great many cases it’s not a good idea.

------
hathawsh
Are the examples in the advisory statement slightly incorrect? The first
example seems to have the user name and host reversed:

    
    
        myhost alice = (ALL) /usr/bin/id
    

All the examples I've seen of sudoers files do it this way:

    
    
        alice myhost = (ALL) /usr/bin/id
    

This is important because the host is rarely used; the host field is usually
replaced with ALL, meaning the host name is not important for the rule:

    
    
        alice ALL = (ALL) /usr/bin/id
    

I hope this isn't some new sudoers syntax.

As I consider whether this bug impacts my company, I see two types of rules in
our sudoers files: (1) rules that let already-privileged users do privileged
things and (2) rules that let processes with minimal privileges make an
exception to normal security rules. This bug doesn't impact rules for highly
privileged users because they already have many ways to do whatever they want.
This bug doesn't impact the second type of rules either because those rules
specify exactly which user to change to; I tested the '-u#-1' trick with one
of those rules on an unpatched sudo and sudo didn't allow it.

The behavior I observed seems to match the advisory, which says the
exploitable rules are those that don't specify a specific user to run as.

Now I wonder: what kind of well-written rule would be exploitable?

------
theamk
I cannot imagine why would one write (ALL, !root) policy -- this seems like a
security hole waiting to happen. There are many system users, and I would not
be surprised if some of them can be escalated to root.

~~~
6c696e7578
Some people do try en enumerate all bad conditions, I have seen this with my
own eyes. cp /bin/bash myehell, sudo meshell. I have never seen anyone try to
enumerate the user part. I'll be waiting with popcorn.

------
compressedgas
The fix was done in
[https://www.sudo.ws/repos/sudo/rev/83db8dba09e7](https://www.sudo.ws/repos/sudo/rev/83db8dba09e7)

The fix appears to be to reject -1 as invalid.

The article should have included in the fix section a link to the commit and a
summary of what the fix was.

------
aargh_aargh
Status in Debian: [https://security-
tracker.debian.org/tracker/CVE-2019-14287](https://security-
tracker.debian.org/tracker/CVE-2019-14287)

Ubuntu: [https://people.canonical.com/~ubuntu-
security/cve/2019/CVE-2...](https://people.canonical.com/~ubuntu-
security/cve/2019/CVE-2019-14287.html)

RHEL:
[https://access.redhat.com/security/cve/cve-2019-14287](https://access.redhat.com/security/cve/cve-2019-14287)

------
andrewchoi
This seems pretty serious, but I wonder if there is a way to measure how many
systems are affected by this. Does anyone have telemetry on how many Runas
configs are set up this way? How would someone collect this data?

~~~
snagglegaggle
In my experience not many people are aware of the permission model of sudo and
I expect a lot of users are merely granted root access. At the same time those
deployments which _do_ use sudo's permission model are likely high cost.

------
nneonneo
Interesting bug; I wonder what else might be affected by setuid(-1). That
said, I’d hope this bug doesn’t really affect too many systems - letting
someone run commands as any non-root user is pretty hazardous because some
users have really high privileges (e.g. any user in the “docker” group is
functionally equivalent to root if Docker is installed).

I do love that this command has its own website - and a delightfully on-point
XKCD-inspired logo :)

~~~
6c696e7578
> I do love that this command has its own website - and a delightfully on-
> point XKCD-inspired logo :)

sudo.ws belongs to Todd, the author of sudo.

------
JetSpiegel
At least the alternative (PolicyKit) reined in on the complexity and is just
executing Javascript...

------
CodeWriter23
Anyone have any idea if someone can just install the old version in their home
directory and enjoy the bypass?

~~~
daxelrod
Sudo works by being setuid root. You need some way of becoming root to set
that in the first place.

~~~
CodeWriter23
Thanks for answering my question.

