OSEC

Neohapsis is currently accepting applications for employment. For more information, please visit our website www.neohapsis.com or email hr@neohapsis.com
 
Subject: Re: SysVinit audit: mesg and wall
From: Solar Designer (solarfalse.com)
Date: Wed Aug 30 2000 - 07:31:20 CDT

Hi,

> I've started auditing SysVinit 2.78 (the latest version as far as I'm aware).

I've spent some time on this about a month ago, so you could want to
have a look at the set of patches we have in Owl (our entire set of
packages isn't public, yet):

        ftp://ftp.openwall.com/pvt/SysVinit-owl-patches.tar.gz

Most of the added bounds checking is there just in case, but it would
still be nicer to also check for truncation on the snprintf()'s which
produce pathnames and other critical strings. This is still to be done.

There're some printf format fixes, including one in init.

> Last night I worked on `mesg' and `wall' and found a few problems.
>
> wall:

We're not going to support SUID/SGID installation of wall, or at
least not now and definitely not by default, so most of what you
mention isn't fixed by our patches.

> * Undocumented -n option suppresses the banner that shows user name, tty,
> etc. Solution: only users listed in wall.c, and the superuser, have access
> to this option. See source comments for details.
>
> * Restricted messages to a customisable number of lines in length to stop
> users flooding the terminal of another user (it doesn't do this very well,
> it's a side effect) and to stop the banner scrolling off the top of the
> terminal. Imagine this:
> $ wall
> [user hits enter key 25 times or more]
> Broadcast message from root (tty1) Tue Aug 29 23:44:46 2000...
>
> You're all fired.
> [eof]
>
> All online users with messages enabled would then see a message that
> looks like it's from root.

Thanks for reminding me of this issue, -- it also affects write.

> * Weird AEROSMITH #define that sends a strange message to a user tyler
> if he's online. I know it's a joke, but it's completely unnecessary :)
> I removed the #define but didn't remove the joke.
>
> * Tiny buffer overflow, \r\n strcat()'d to the end of a buffer that may not
> have enough room left in it. I made the buffer bigger.

This strcat() we had fixed, just for the reliability.

- if (len >= MAXLEN) break;
+ if (len >= MAXLEN - 2) break;

> mesg:
>
> * Will set tty world writeable if it's not owned by group `tty' and messages
> are turned on with `mesg y'. My solution is to only ever play with the
> group mode bits and warn if the tty does not have the correct group.

I think we're going to take your patch for this.

> As a side note, utmpdump is quite buggy and will strcpy() user input to fixed
> length strings without first checking the length. This isn't a security
> problem unless a malicious user tricks another user into `undumping' bad data
> with `utmpdump -r'.

Yes, this is a problem.

> shutdown can run setuid and calls system() in a few times
> if certain options are given and does some suspiscious strcpy()'s; I wouldn't
> consider it safe to run setuid, but I haven't had time to analyse it yet.

I don't think it would make sense to patch this version of shutdown
for safe SUID installation (and I don't need that, anyway).

> I'll look at the rest of SysVinit eventually, this is just a start. Feedback
> on the attached patches would be greatly appreciated. They're also
> available at:
> http://box3n.gumbynet.org/~fyre/

Signed,
Solar Designer