Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Potential bypass of Runas user restrictions in sudo

sudo.ws

Post reply on HN
Print view View original

11–20 of 29 posts

Re: Potential bypass of Runas user restrictions in sudo

#11

Post by wbkang »

gambler wrote: post #8

Is this the official sudo repository? 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?

No, it is not very good. I remember this one particular example that I was surprised to see years ago, and I actually encountered the bug at work. There is a constant named MAX_UID_T_LEN to denote the maximum number of characters that a UID can have. For whatever reason, LDAP-synced UIDs at work had many digits and sudo would fail to work because the code only worked UIDs that have MAX_UID_T_LEN - 1 characters.

The bug was fixed in 2012 - https://www.sudo.ws/changes.html: Use MAX_UID_T_LEN + 1 for uid/gid buffers, not MAX_UID_T_LEN to prevent potential truncation. Bug #562.

Re: Potential bypass of Runas user restrictions in sudo

#12

Post by notacoward »

gambler wrote: post #8

Is this the official sudo repository? 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?

Welcome to the world of code older than most people on HN. Sudo is about 30 years old. A lot of code from that era, including the kernels and utilities for most flavors of UNIX, was written in a similar style. The "goto done" idiom was a pretty common form of error handling in Plain Old C, and was more readable/verifiable than some alternatives such as deeply nested functions. Remember, this is C - no objects (so no destructors for RAII), no exceptions, etc. The lack of unit tests is explainable by the fact that tests were often external to the running code and considered proprietary. It's all too easy for people who never had to deal with serious portability issues or even know what threats existed in the tty-handling stack to hold code like this up against a standard that was still decades in the future and find it wanting. Is it useful?

Re: Potential bypass of Runas user restrictions in sudo

#15

Post by gambler »

jblow wrote: post #9
gambler wrote: post #8

Is this the official sudo repository? 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?

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.

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.

Re: Potential bypass of Runas user restrictions in sudo

#16

Post by yjftsjthsd-h »

jblow wrote: post #9
gambler wrote: post #8

Is this the official sudo repository? 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?

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.

There is a reason the OpenBSD folks decided to write doas:)

Re: Potential bypass of Runas user restrictions in sudo

#18

Post by 6c696e7578 »

theamk wrote: post #17

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.

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.

Re: Potential bypass of Runas user restrictions in sudo

#19

Post by 6c696e7578 »

nneonneo wrote: post #3

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 delightfull…

> 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.

Post reply on HN