Re: enforcing RLIMIT_NPROC in setuid() ?

[christos%astron.com@localhost (Christos Zoulas)]

In article <20080110184357.GA24955%panix.com@localhost>,
Ed Ravin  <eravin%panix.com@localhost> wrote:
>One of my customers found an interesting way around RLIMIT_NPROC the
>other day - he was running a script from another site that ssh'd in,
>and due to bad coding in the script it began running an infinite loop
>of ssh calls, all of which stayed logged in at the same time.
>
>The result was that the customer was logged in hundreds of times
>on the same machine, with well over the default 160 processes set
>by "ulimit -n".  He eventually reached MAXNPROC and the system was
>unable to do anything useful until we figured out what was going on
>and stopped his script from logging in.
>
>I believe the reason this happened was because sshd starts
>out as root, and once authentication is complete, demotes itself
>to run as the user logging in (presumably by using setuid() and/or
>seteuid() ).
>
>Would it make sense to have setuid() check the process limit,
>and return an error if the user in question is over the limit?
>That way, programs that check the return value of setuid() would
>fail and prevent new processes from being created by login loops
>like the one described above.
>
>It would probably be a good idea for setuid() to still carry out
>the UID change, just in case there's a poorly written program
>somewhere that doesn't check the return value.

I think that this has been discussed:
http://www.mail-archive.com/freebsd-hackers%freebsd.org@localhost/msg08638.html
http://www.bress.net/blog/archives/34-setuid-madness.html

The problem is that if you carry the UID change, then you effectively
exceed the limit so if the program does not check the return value
you end up back to square one. If you don't change the UID, then
you can cause security problems. Another idea is to signal the
process similarly to what is done with XCPU, but this is ugly and
non-portable. Yet another is to do the checking on exec, i.e. let
root fork() and setuid(), but enforce the limit on exec().

christos