Re: enforcing RLIMIT_NPROC in setuid() ?

[Ed Ravin <eravin%panix.com@localhost>]

On Thu, Jan 10, 2008 at 07:19:44PM +0000, Christos Zoulas wrote:
> 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 [...]

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

That's what I was thinking.  It does no harm to have setuid() return
an error if it still carries out the UID change - properly-written
programs would error off immediately, dumb programs would do what
they did before and carry on as the expected non-root user, without
any new security problems (unlike the Linux example referred to above,
where the program ran as root while it thought it had changed to non-root)..

> Yet another is to do the checking on exec, i.e. let
> root fork() and setuid(), but enforce the limit on exec().

This is more effective, since it will catch all of the common cases,
like sshd and cron.  It should catch all the cases of accidental
overflow of the process limit, as my customer did.

But a program could be taking actions after setuid() and before exec(),
such as reading in a user-created initialization file.  Modifying
setuid() gives us a chance to catch those too, but only a chance
since the program could ignore the return value.

Should I open a up a PR on this?