[PATCH] wall: Don't allow reading of files the real user might not have access to

Rich Felker dalias at aerifal.cx
Mon Sep 30 14:44:19 UTC 2013


On Mon, Sep 30, 2013 at 01:53:58AM -0400, Mike Frysinger wrote:
> On Sunday 29 September 2013 21:54:34 Rich Felker wrote:
> > On Mon, Sep 30, 2013 at 08:30:01AM +1000, Ryan Mallon wrote:
> > > The wall applet is setuid and currently does no checking of the real
> > > user's read access to the message file. This allows the wall applet
> > > to be used to display files which are not readable by an
> > > 
> > > unprivileged user. For example:
> > >   $ wall /etc/shadow
> > >   $ wall /proc/vmallocinfo
> > > 
> > > Fix this issue by introducing the same check as used by the
> > > util-linux version of wall: only allow the file argument to be used
> > > if the user is root, or the real and effective uid/gids are equal.
> > 
> > No, the fix is to make it so wall is not one of the suid applets.
> > There is no reason whatsoever for wall to be suid. Users who want wall
> > messages are supposed to make their terminal world-writable. Users who
> > don't (i.e. anyone sane) makes their terminal non-world-writable.
> 
> busybox emulates the "real world" utils.  `wall` (in sysvinit and util-linux) 
> is setgid (where group==tty), hence busybox follows suite.

sgid-tty and suid-root are quite different. The impact of this bug
would be a lot less if busybox were actually following this historical
practice and taking up egid=tty then dropping root before doing
anything. It would also work with the other historical tty permissions
option where the tty's owner is the user, its group is "tty", and
permissions are either 620 or 600 depending on whether the user wants
wall-spam or not.

> requiring users to make their terminal world-writable is a way worse idea.

Nobody is requiring anybody to use wall. The best part about the way I
said it should be configured is that you don't get wall spam by
default (unless root is sending it, in which case it's at least
somewhat valid).

> > Idiotic bugs like this (utilities which have no business being suid
> > getting treated as one of the suid cases) are why I recommend not even
> > using the busybox suid feature at all, or at least making a separate
> > busybox binary with only the should-be-suid applets compiled in.
> 
> not really relevant.  if you had a build of busybox that had just wall, it'd 
> still have the bug Ryan mentioned.

No. wall would not be included in the list of applets for the suid
binary; only things like su and ping, which are actually supposed to
be suid-root, would be in the suid buinary.

Rich


More information about the busybox mailing list