Subject: Re: ktrace and P_SUGID
To: None <itojun@iijlab.net>
From: Robert Elz <kre@munnari.OZ.AU>
List: tech-security
Date: 06/30/2002 12:25:21
    Date:        Sun, 30 Jun 2002 03:26:02 +0900
    From:        itojun@iijlab.net
    Message-ID:  <20020629182602.9DB3B4B27@coconut.itojun.org>

  | 	there are other uses of "cr_uid == 0" in sys/kern.  could you
  | 	check if any of these are incorrect or not?
  | 	(obviously, the one within suser() is okay)

The obvious one, and the other one in ktrace.c are OK as they are.
The other 3 are incorrect.

Changing CANSIGNAL() is easy - just means moving the modified test to the end
of the macro, rather than at the start (the macro has the parameters needed
for the call to suser already available).

The other two are much harder to fix, as a call to suser() is only valid
after it has been determined that the user couldn't have performed the
operation anyway - that is, it is using uid==0 priv to override what
would otherwise have been OK.   Those other two are using the check to
short circuit all the normal permission checks, and because it is written
that way, there's no real easy place to put a suser() call (I guess
it could be put in each other possible exit point, so instead of
(for example)

	   return ((perm->mode & mask) == mask ? 0 : EACCES);

the code would be

           return ((perm->mode & mask) == mask || suser(...)==0 ? 0 : EACCES);

But those probably really aren't worth the bother of fixing.

kre