Date: Thu, 2 Mar 2000 18:26:22 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Kris Kennaway <kris@hub.freebsd.org> Cc: current@FreeBSD.ORG Subject: Re: HEADS UP! IPC security (Re: cvs commit: src/sys/kern sysv_ipc.c (fwd)) Message-ID: <Pine.BSF.4.21.0003021742060.1078-100000@alphplex.bde.org> In-Reply-To: <Pine.BSF.4.21.0003012040060.4968-100000@hub.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> ---------- Forwarded message ---------- > Date: Wed, 1 Mar 2000 21:03:22 -0500 (EST) > From: Brian Dean <brdean@unx.sas.com> > To: kris@freebsd.org > Cc: phk@freebsd.org > Subject: Re: cvs commit: src/sys/kern sysv_ipc.c > > [SNIP - KK] > > The bug is very easily tested/verified: just create a semaphore, then, > as root, try to delete it. If the bug is there, you will get > "Operation not permitted". Then try to delete it using an account > other than the one that created it, which is supposed to fail, but > works fine. > > It appears that it might have cropped up at version 1.8 > sys/kern/sysv_ipc.c. At version 1.8, the check: > > if (cred->cr_uid == 0) > > was replaced with: > > if (suser(cred, (u_short *)NULL)) It had rotted further since then (as half threatened in rev.1.9) to passing the process pointer so that it can mess up p->p_acflag. This results in the ASU flag always being set in p->p_acflag for root, although no special privilege is required for root to operate on ipc objects owned by root, and especially, no special privilege is required to determine whether an operation is permitted. The ASU flag should only be set if root privilege is used. Most callers of ipcperm() are committed to doing the operation that they check for using ipcperm() if ipcperm() succeeds, so rearrangement of ipcperm() to check for root privilege last would fix most cases. The corresponding code in ufs_access() uses the (cred->cr_uid == 0) check. ufs_access() is used internally (via VOP_ACCESS()) in much the same way as ipcperm(). This seems to give the opposite bug in many callers -- ASU is not set when root privilege is used. Rearranging ufs_access() to check for root privilege last wouldn't help, because ufs_access() shouldn't set ASU -- it is used for the access() system call which doesn't use any special privilege. Now the code has rotted to having an almost unused variable and one other style bug: int error; ... error = suser(p); if (!error) This should be written much like the check in rev.1.8: if (suser(p) == 0) Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0003021742060.1078-100000>