public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: question about /proc/<PID>/mem in 2.4 (fwd)
@ 2004-07-07 13:28 Tigran Aivazian
  2004-07-07 23:48 ` Solar Designer
  0 siblings, 1 reply; 11+ messages in thread
From: Tigran Aivazian @ 2004-07-07 13:28 UTC (permalink / raw)
  To: solar; +Cc: Alan Cox, Marcelo Tosatti, linux-kernel

just another attempt, in hope that perhaps the extra "dot" in the email 
address was actually meant to be "." to prevent spam.

Kind regards
Tigran

---------- Forwarded message ----------
Date: Wed, 7 Jul 2004 14:53:44 +0100 (BST)
From: Tigran Aivazian <tigran@aivazian.fsnet.co.uk>
To: solar@openwall.dot.com
Cc: Alan Cox <alan@redhat.com>, Marcelo Tosatti <marcelo.tosatti@cyclades.com>,
     linux-kernel@vger.kernel.org
Subject: Re: question about /proc/<PID>/mem in 2.4

On Tue, 6 Jul 2004, Alan Cox wrote:

> On Tue, Jul 06, 2004 at 02:08:04PM +0100, Tigran Aivazian wrote:
> > > This code was added to stop the ptrace/kmod vulnerabilities. I do not 
> > > fully understand the issues around tsk->is_dumpable and the fix itself,
> > > but I agree on that the checks here could be relaxed for the super user.
> 
> is_dumpable tells you various things in 2.4, including whether the
> curent memory image is valid, and as a race preventor for ptrace during
> exec of setuid apps. You should probably talk to Solar Designer about
> the whole design of the dump/suid race fixing work rather than me.
> 
> We also had to deal with another nasty case which could be fixed by grabbing
> the mm at open time (which then opens a resource attack bug).
> 
> Consider what happens if your setuid app reads stdin
> 
> 	setuidapp < /proc/self/mem
> 
> (No idea how 2.6 deals with these but if its got better backportable ways
>  that *actually work* it might make sense).

Hello,

Ok, I followed the advice of Alan Cox and looked up "Solar Designer" on 
google.

So, my questions (to Solar Designer) are:

1. what exactly are the details of the setuid race Alan Cox is referring
to above? I have 0 (zero) doubts in the validity of Alan's words and
always trust him, but it doesn't imply that I always understand him, and
so would appreciate a clarification.

2. are you sure that there is no way to fix the above race without
preventing even the superuser access to /proc/<PID>/mem of 
any process?

More specifically (to save you time re-reading this thread) I am referring 
to the code in fs/proc/base.c:mem_read():

        if (!MAY_PTRACE(task) || !may_ptrace_attach(task))
                return -ESRCH;

and also the same check on return from access_process_vm(), which I 
suggested to relax a little, by allowing CAP_SYS_PTRACE user (or 
CAP_SYS_RAWIO perhaps?) to access /proc/<PID>/mem of any task without any 
hindrance.

Btw, the second check looks like an obvious race to me. I.e. if this
condition can change between the check in the beginning of mem_read() and
return from access_process_vm() then what is to stop it from changing just
after this second check and return from mem_read()?

Therefore, the code in mem_read() does look broken to me, but instead of 
"re-inventing the wheel" and trying to fix it "from scratch" I would like 
to get a fair knowledge of the history/reasons for these changes, please.

Kind regards
Tigran

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about /proc/<PID>/mem in 2.4 (fwd)
  2004-07-07 13:28 question about /proc/<PID>/mem in 2.4 (fwd) Tigran Aivazian
@ 2004-07-07 23:48 ` Solar Designer
  2004-07-18 12:41   ` Tigran Aivazian
  0 siblings, 1 reply; 11+ messages in thread
From: Solar Designer @ 2004-07-07 23:48 UTC (permalink / raw)
  To: Tigran Aivazian; +Cc: Alan Cox, Marcelo Tosatti, linux-kernel

On Wed, Jul 07, 2004 at 02:28:10PM +0100, Tigran Aivazian wrote:
> So, my questions (to Solar Designer) are:
> 
> 1. what exactly are the details of the setuid race Alan Cox is referring
> to above? I have 0 (zero) doubts in the validity of Alan's words and
> always trust him, but it doesn't imply that I always understand him, and
> so would appreciate a clarification.

The problem is the process the /proc/<PID>/mem of which you're reading
may be ptrace-detached and proceed into a SUID/SGID exec while you're
still inside mem_read().  This was the reason for my addition of the
may_ptrace_attach() function and its uses in mem_read() in 2.4.21-ow2
and 2.4.22.  Here's an excerpt from an e-mail I wrote at the time:

| Date: Sun, 6 Jul 2003 05:37:15 +0400
| From: Solar Designer <solar at openwall.com>
| To: Paul Starzetz <paul at starzetz.de>
| Cc: Alan Cox <alan at redhat.com>
| Subject: Re: [vendor-sec] Linux /proc sensitive information disclosure
| [...]
| ...reviewing mem_read() I see potential ways to bypass
| its additional checks.
| 
| MAY_PTRACE() is only checked once and I don't see anything which would
| prevent the task from being ptrace-detached and continuing with SUID
| exec as mem_read() proceeds with access_process_vm().
| 
| I'm not sure the use of self_exec_id is appropriate.  What if an fd
| for an opened mem entry is transferred (over Unix domain socket) into
| another process?  Shouldn't this be self_exec_id for the target task
| and not current anyway?
| 
| There may be reasons why the existing checks are sufficient (I just
| don't see them), but I'd feel more comfortable if mem_read() kept a
| lock on the task preventing it getting replaced on exec and/or if it
| re-checked permissions after the access_process_vm() but before the
| copy_to_user().

Re-checking permissions is what I had implemented.

> 2. are you sure that there is no way to fix the above race without
> preventing even the superuser access to /proc/<PID>/mem of 
> any process?

I think this is a different issue.  You seem to be complaining about
the older MAY_PTRACE() check, not about the added may_ptrace_attach()
check.  Alan has already pointed out a reason why the MAY_PTRACE()
check was needed:

| Consider what happens if your setuid app reads stdin
| 
| 	setuidapp < /proc/self/mem

> More specifically (to save you time re-reading this thread) I am referring 
> to the code in fs/proc/base.c:mem_read():
> 
>         if (!MAY_PTRACE(task) || !may_ptrace_attach(task))
>                 return -ESRCH;
> 
> and also the same check on return from access_process_vm(), which I 
> suggested to relax a little, by allowing CAP_SYS_PTRACE user (or 
> CAP_SYS_RAWIO perhaps?) to access /proc/<PID>/mem of any task without any 
> hindrance.

See Alan's example I've quoted above.  In this scenario, it would be
the program being attacked which will be checked for possession of the
capability... if it is SUID root, the attack will succeed.

As for the checks performed inside may_ptrace_attach(), they're
derived from ptrace itself and are due to numerous attacks which
otherwise were possible on ptrace.  /proc/<PID>/mem is very similar.

> Btw, the second check looks like an obvious race to me. I.e. if this
> condition can change between the check in the beginning of mem_read() and
> return from access_process_vm() then what is to stop it from changing just
> after this second check and return from mem_read()?

I don't understand the attack you're describing here.  If anything
changes after mem_read() has already returned, then it's not our
business: the process has obtained a copy of data it had legitimate
access to.

-- 
Alexander Peslyak <solar@openwall.com>
GPG key ID: B35D3598  fp: 6429 0D7E F130 C13E C929  6447 73C3 A290 B35D 3598
http://www.openwall.com - bringing security into open computing environments

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about /proc/<PID>/mem in 2.4 (fwd)
  2004-07-07 23:48 ` Solar Designer
@ 2004-07-18 12:41   ` Tigran Aivazian
  2004-07-18 12:59     ` Solar Designer
  2004-07-23 17:34     ` Alan Cox
  0 siblings, 2 replies; 11+ messages in thread
From: Tigran Aivazian @ 2004-07-18 12:41 UTC (permalink / raw)
  To: Solar Designer; +Cc: Alan Cox, Marcelo Tosatti, linux-kernel

Hi,

Thank you for your reply, but this one bit still remains utterly unclear 
to me:

> Alan has already pointed out a reason why the MAY_PTRACE()
> check was needed:
> 
> | Consider what happens if your setuid app reads stdin
> | 
> | 	setuidapp < /proc/self/mem
> 
> ... 
> See Alan's example I've quoted above.  In this scenario, it would be
> the program being attacked which will be checked for possession of the
> capability... if it is SUID root, the attack will succeed.

In the above example there is nothing forbidden and the current state of 
things doesn't prevent the program from reading it's own address space.

Thus I see absolutely nothing special about the case:

# setuidapp < /proc/self/mem

and this program reading stdin. Maybe I am missing something obvious but I
have 10+ years of Unix systems programming experience and having consulted
some people who have 20+ years of such experience they are also of the
same opinion, i.e. nothing special in the above case.

Could you therefore clarify it, please? Thank you in advance!

Kind regards
Tigran


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about /proc/<PID>/mem in 2.4 (fwd)
  2004-07-18 12:41   ` Tigran Aivazian
@ 2004-07-18 12:59     ` Solar Designer
  2004-07-18 21:27       ` Willy Tarreau
  2004-07-23 17:34     ` Alan Cox
  1 sibling, 1 reply; 11+ messages in thread
From: Solar Designer @ 2004-07-18 12:59 UTC (permalink / raw)
  To: Tigran Aivazian; +Cc: Alan Cox, Marcelo Tosatti, linux-kernel

On Sun, Jul 18, 2004 at 01:41:34PM +0100, Tigran Aivazian wrote:
> > | 	setuidapp < /proc/self/mem
[...]
> In the above example there is nothing forbidden and the current state of 
> things doesn't prevent the program from reading it's own address space.
> 
> Thus I see absolutely nothing special about the case:
> 
> # setuidapp < /proc/self/mem
> 
> and this program reading stdin.

The problem is the program does not know its stdin corresponds to a
process' address space and it does not know it is making use of a
privilege to read it.  A correctly written SUID root program may
reasonably assume that the data it obtains from stdin is whatever the
unprivileged user has provided, -- and, for example, display such data
back to the user (as a part of an error message or so).  If we permit
reads from /proc/<pid>/mem based on credentials of the read(2)-calling
process only, this assumption would be violated resulting in security
holes.

Oh, by the way, I've just noticed that the above example is not
entirely correct.  In order to read setuidapp's own address space
(after the kernel has been patched according to your proposal), it
should have been:

$ exec setuidapp < /proc/self/mem

-- 
Alexander Peslyak <solar@openwall.com>
GPG key ID: B35D3598  fp: 6429 0D7E F130 C13E C929  6447 73C3 A290 B35D 3598
http://www.openwall.com - bringing security into open computing environments

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about /proc/<PID>/mem in 2.4 (fwd)
  2004-07-18 12:59     ` Solar Designer
@ 2004-07-18 21:27       ` Willy Tarreau
  2004-07-18 23:15         ` Paul Jackson
  0 siblings, 1 reply; 11+ messages in thread
From: Willy Tarreau @ 2004-07-18 21:27 UTC (permalink / raw)
  To: Solar Designer; +Cc: Tigran Aivazian, Alan Cox, Marcelo Tosatti, linux-kernel

Hi !

On Sun, Jul 18, 2004 at 04:59:25PM +0400, Solar Designer wrote:
> On Sun, Jul 18, 2004 at 01:41:34PM +0100, Tigran Aivazian wrote:
[...]
> > # setuidapp < /proc/self/mem
> > 
> > and this program reading stdin.
> 
> The problem is the program does not know its stdin corresponds to a
> process' address space and it does not know it is making use of a
> privilege to read it.  A correctly written SUID root program may
> reasonably assume that the data it obtains from stdin is whatever the
> unprivileged user has provided, -- and, for example, display such data
> back to the user (as a part of an error message or so).  If we permit
> reads from /proc/<pid>/mem based on credentials of the read(2)-calling
> process only, this assumption would be violated resulting in security
> holes.

unless I'm mistaken, it will be the shell which will open /proc/self/mem,
so if it doesn't have correct rights, even the setuidapp will not have
access to it because the shell will not be able to open the file.

> Oh, by the way, I've just noticed that the above example is not
> entirely correct.  In order to read setuidapp's own address space
> (after the kernel has been patched according to your proposal), it
> should have been:
> 
> $ exec setuidapp < /proc/self/mem

Hmm, this is an interesting case. What exactly is passed then ? You agree
that the shell opens its own memory map on fd 0 then execs setuidapp
which will have it on stdin. But will the fd contents be automatically
replaced with the new process' memory map or will it still be the shell's
until this last reference is dropped ?

Cheers,
Willy


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about /proc/<PID>/mem in 2.4 (fwd)
  2004-07-18 21:27       ` Willy Tarreau
@ 2004-07-18 23:15         ` Paul Jackson
  2004-07-19  4:54           ` Willy Tarreau
  2004-07-22 20:34           ` Alan Cox
  0 siblings, 2 replies; 11+ messages in thread
From: Paul Jackson @ 2004-07-18 23:15 UTC (permalink / raw)
  To: Willy Tarreau; +Cc: solar, tigran, alan, marcelo.tosatti, linux-kernel

> What exactly is passed then ...

The patch /proc/self/mem will be evaluated just once, on the open
by the original shell.  Whatever bucket of bits that resolves to
will remain the source for fd == 0 reads.

That original shell's mem file will be read by whatever follows, exec or
not.  The 'exec' just stops the shell from forking before it exec's, and
certainly won't cause the path that was used earlier to open fd 0 to be
re-evaluated.

The setuidapp will see the shell's memory.  In general, a app, setuid or
not, should make no assumption that any open fd's handed to it at birth
were opened using the same priviledges that the app itself has.

Or, more to the point, no higher priviledge app, when calling down to a
lessor priviledge app (say a setuid or root app invoking a less trusted
app) should allow any open file descriptors across the fork or exec,
except those open on files to which it determines the lessor context has
rights.

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about /proc/<PID>/mem in 2.4 (fwd)
  2004-07-18 23:15         ` Paul Jackson
@ 2004-07-19  4:54           ` Willy Tarreau
  2004-07-19  6:47             ` Paul Jackson
  2004-07-22 20:34           ` Alan Cox
  1 sibling, 1 reply; 11+ messages in thread
From: Willy Tarreau @ 2004-07-19  4:54 UTC (permalink / raw)
  To: Paul Jackson; +Cc: solar, tigran, alan, marcelo.tosatti, linux-kernel

On Sun, Jul 18, 2004 at 04:15:49PM -0700, Paul Jackson wrote:
> That original shell's mem file will be read by whatever follows, exec or
> not.  The 'exec' just stops the shell from forking before it exec's, and
> certainly won't cause the path that was used earlier to open fd 0 to be
> re-evaluated.

I totally agree, of course, but...

> The setuidapp will see the shell's memory.  In general, a app, setuid or
> not, should make no assumption that any open fd's handed to it at birth
> were opened using the same priviledges that the app itself has.

how can you be sure it will be the shell's memory ? after an exec, the
new process replaces the shell with the same pid. If it overwrites the
same address space, there's a possibility that /proc/self/mem, once
openned, still points to the same structure which will reflect the new
process's space after exec(). I'm afraid I'll have to test it.

Regards,
Willy


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about /proc/<PID>/mem in 2.4 (fwd)
  2004-07-19  4:54           ` Willy Tarreau
@ 2004-07-19  6:47             ` Paul Jackson
  0 siblings, 0 replies; 11+ messages in thread
From: Paul Jackson @ 2004-07-19  6:47 UTC (permalink / raw)
  To: Willy Tarreau; +Cc: solar, tigran, alan, marcelo.tosatti, linux-kernel

Willy writes:
> new process replaces the shell with the same pid.
> ... I'm afraid I'll have to test it.

True - same pid - good point.  Yup - testing is in order.

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about /proc/<PID>/mem in 2.4 (fwd)
  2004-07-18 23:15         ` Paul Jackson
  2004-07-19  4:54           ` Willy Tarreau
@ 2004-07-22 20:34           ` Alan Cox
  2004-07-22 21:23             ` Paul Jackson
  1 sibling, 1 reply; 11+ messages in thread
From: Alan Cox @ 2004-07-22 20:34 UTC (permalink / raw)
  To: Paul Jackson
  Cc: Willy Tarreau, solar, tigran, alan, marcelo.tosatti, linux-kernel

On Sun, Jul 18, 2004 at 04:15:49PM -0700, Paul Jackson wrote:
> The setuidapp will see the shell's memory.  In general, a app, setuid or
> not, should make no assumption that any open fd's handed to it at birth
> were opened using the same priviledges that the app itself has.


Now think about "exec suidapp < ..." thats more fun


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about /proc/<PID>/mem in 2.4 (fwd)
  2004-07-22 20:34           ` Alan Cox
@ 2004-07-22 21:23             ` Paul Jackson
  0 siblings, 0 replies; 11+ messages in thread
From: Paul Jackson @ 2004-07-22 21:23 UTC (permalink / raw)
  To: Alan Cox; +Cc: willy, solar, tigran, alan, marcelo.tosatti, linux-kernel

Alan wrote:
> ... thats more fun

Uh - yeah.

As I acknowledged to Willy a couple of days ago ... good point.

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about /proc/<PID>/mem in 2.4 (fwd)
  2004-07-18 12:41   ` Tigran Aivazian
  2004-07-18 12:59     ` Solar Designer
@ 2004-07-23 17:34     ` Alan Cox
  1 sibling, 0 replies; 11+ messages in thread
From: Alan Cox @ 2004-07-23 17:34 UTC (permalink / raw)
  To: Tigran Aivazian; +Cc: Solar Designer, Alan Cox, Marcelo Tosatti, linux-kernel

On Sun, Jul 18, 2004 at 01:41:34PM +0100, Tigran Aivazian wrote:
> > | 	setuidapp < /proc/self/mem
> > 
> > ... 
> > See Alan's example I've quoted above.  In this scenario, it would be
> > the program being attacked which will be checked for possession of the
> > capability... if it is SUID root, the attack will succeed.
> 
> In the above example there is nothing forbidden and the current state of 
> things doesn't prevent the program from reading it's own address space.

I meant to say exec setuidapp </proc/self/mem



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2004-07-23 17:35 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-07-07 13:28 question about /proc/<PID>/mem in 2.4 (fwd) Tigran Aivazian
2004-07-07 23:48 ` Solar Designer
2004-07-18 12:41   ` Tigran Aivazian
2004-07-18 12:59     ` Solar Designer
2004-07-18 21:27       ` Willy Tarreau
2004-07-18 23:15         ` Paul Jackson
2004-07-19  4:54           ` Willy Tarreau
2004-07-19  6:47             ` Paul Jackson
2004-07-22 20:34           ` Alan Cox
2004-07-22 21:23             ` Paul Jackson
2004-07-23 17:34     ` Alan Cox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox