Re: [SECURITY] /proc/$pid/ leaks contents across setuid exec

Kees Cook
2011-02-10T06:38:54Z

On Wed, Feb 09, 2011 at 07:41:41PM -0800, Eric W. Biederman wrote:
> James Morris <jmorris@namei.org> writes:
>
> > On Tue, 8 Feb 2011, Eric W. Biederman wrote:
> >
> >> Kees Cook <kees.cook@canonical.com> writes:
> >>
> >> > On Tue, Feb 08, 2011 at 02:43:15PM +1100, James Morris wrote:
> >
> >> >> > I don't think /proc/$pid/* needs to stay open across execs, does it? Or at
> >> >> > least the non-0444 files should be handled separately.
> >> >>
> >> >> Actually, this seems like a more general kind of bug in proc rather than a
> >> >> leaked fd. Each child task should only see its own /proc/[pid] data.
> >> >
> >> > Right, that's precisely the problem. The unprivileged process can read
> >> > the setuid process's /proc files.
> >>
> >> If these are things that we actually care about we should sprinkle in a
> >> few more ptrace_may_access calls into implementations of the relevant
> >> proc files.
> >
> > This seems to be papering over a bug.
> >
> > It is plainly broken to return an access error to a task which is
> > legitimately accessing a file. The task should not receive the wrong
> > information from /proc/[pid]/* .
>
> Per task files are special because of exec. The permission needed
> change dynamically. The common solution to this problem (see ttys) is
> to revoke anyone who has file descriptors open. Proc does something a
> little different and simply gives you a permission error when you read
> or write if it would be a problem.
>
> We happen to call the test to see if you should have permission
> security_may_ptrace because ptrace lets you get the information anyway
> so we might as well allow the information from /proc.
>
> Given that security_may_ptrace is the existing model, and that we don't
> return wrong data, but a clear an unambiguous error I don't see problems
> with the approach.
>
> The practical question is, is the data sensitive enough that we want
> this protection.

This seems reasonable; they're mode 0400 for a reason. The auxv file
alone is a nearly total ASLR offset leak. The may_ptrace() worked well
for /proc/$pid/maps, and it started as 0444 historically and had a lot
of additional carefully managed requirements. Adding the same restriction
to all the already-mode-0400 files seems logical.

-Kees

--
Kees Cook
Ubuntu Security Team