Re: Another nice tmp race

[solar () FALSE COM (Solar Designer)]

Hello,

>         Playing with my new shiny Slackware 3.5 box I have noticed

I must say that I haven't looked at the exact Slackware 3.5's version
of in.pop3d, but I think that it doesn't differ much from the version
I checked a while ago (1.005l). (I was reading lots of different pop3d
sources lately, as I'm now working on a hopefully better one myself;
not sure if reading that much of broken code was of any use though.;-)

> something unusual. The in.pop3d daemon creates sometimes locks for some
> mailboxes in /usr/tmp/.pop. The directory is  drwxrwxrwt so there will be
> no problem in creating nice links to /zImage, /vmlinuz, /etc/shadow or
> whatever comes in your head. Be creative.

You haven't actually tried that, I assume. Fortunately, it is smart enough
to set O_EXCL. There's an obvious DoS though, as the filename is fixed.

Still, I do consider this pop3d to be pretty much broken, for the following
security reasons (in addition the above DoS):

[ 0. The design could be much better. For example, parsing the POP commands
doesn't require root privileges, it's only the mailbox access that requires
a setuid() to the user after authentication. It is possible to reduce the
amount of code running as root significantly, by forking a separate process
for the authentication state. As you might have guessed, the draft version
of my pop3d (not for distribution, yet) does that. :-) ]

1. It looks like the author isn't familiar with the semantics of strncpy()
and strncat(). This can potentially result in the lack of NUL termination
on some strings, and in off-by-one buffer overflows. Both of these might
be exploitable under some circumstances. However, more importantly, this
speaks about the overall quality of the code.

2. The POP command parsing is really awful: every command is strncmp()'d
and parsed separately, with a buffer passed from main() down into other
functions. This can potentially result in hard to track buffer handling
problems. There're several places where we don't run into a problem due
to pure luck. One such example is:
                if( ptr = index(inbuf,'@') ) {
                        strncpy(cli_user,inbuf,ptr-inbuf);
This can leave cli_user totally uninitialized: not even NUL terminated.
I doubt that anyone really wanted to depend on the fact that all of .bss
is zero at startup. Another example, where we, fortunately, don't have a
buffer overflow, is:
                sscanf(inbuf, "%s", cli_user);
Basically, it is hard to audit such code. To check if the above sscanf()
can result in a buffer overflow or not, one has to find where cli_user
is defined, where inbuf can get its value from, and where the buffer(s)
that inbuf (which is a char * pointer) can point to are defined. These
turn out to be all in different places. It is easy to miss security holes
in this kind of code.

3. After authentication, the code drops its root privileges, but doesn't
drop the groups. I wonder why. The supplementary groups I can understand:
good inetd's shouldn't pass them (some do though). However, dropping the
current GID is a must here anyway. Of course, this one is easy to fix.

4. The rm_stale.sh script is full of security and reliability problems of
different kinds. These include: unsafe temporary files, spoofing process
names, O(N**2) execution time (with N under the control of an attacker).
The README suggests to put it into root's crontab.

>         Please notify the authors, I still don't know their email and I
> apologize for that.

I'm CC'ing this to the current maintainter (well, at least I hope so).

By the way, as I'm working on a pop3d myself, I am obviously biased.
No offence intended. :-)

Signed,
Solar Designer