I did some experimenting with xmosiac today on one of my
machines, running it under a trace to see if it does anything
obviously bad. It didn't look like it was doing anything immediately
and clearly unpleasant, but a perusal of the source code makes
me pretty uncomfortable.
+ There is widespread use of system() to invoke the shell
+ There is little or no checking to control spoofing system()
+ There are lots of bits in the code that indicate an unsettling
lack of awareness of how UNIX works
These three observations, combined with the fact that Xmosiac
is a pretty hefty program, makes it hard for me to feel confident that
the "right" things are being handed to system(). Generally, the problems
revolve around cleaning up filenames and so forth -- the filenames are
the tricky part. It's hard to track down what code pathways invoke the
routines that make lots of calls to system() and how the file names
that it uses are generated. Perhaps someone with more familiarity with
the code can comment.
I ran xmosaic for a while under "trace" and saved the output
to a file and grepped it for cases of open() and exec() and so forth.
It seemed to be doing all its I/O to reasonably-named files in /usr/tmp
and wasn't system()ing stuff. Presumably I didn't hit the right code
paths to make it system() stuff. There are *LOTS* of invocations of
system().
Many of the invocations of system() make me nervous since they
show a lack of awareness of how UNIX works. For example, files are
removed using the following:
> cmd = (char *)malloc (sizeof (char) * (strlen (fnam) + 32));
> sprintf (cmd, "/bin/rm -f %s", fnam);
> system (cmd);
> free (cmd);
Rather than using the unlink() system call. Rather unusual
programming practice, especially if someone manages to somehow get
a file named 'foo *' into the filename that's being removed. In other
places syntax like system("mv file1 file2") is used, rather than
relying on the rename() system call.
There's also code like:
> {
> /* For rlogin, we should use -l user. */
> sprintf(command, "%s -e %s %s %s %s %s &", xterm_str, access,
> hostname,
> port ? port : "",
> user ? "-l" : "",
> user ? user : "");
> }
> else
> {
> /* For telnet, -l isn't safe to use at all -- most platforms
> don't understand it. */
> sprintf(command, "%s -e %s %s %s &", xterm_str, access,
> hostname,
> port ? port : "");
> }
> system(command);
What's unsettling about this stuff is that it runs on
your internal workstation, within your protected perimeter. So,
if you're using a SOCKSified mosiac client, someone can't break
into the firewall, but they may still be perfectly able to do
damage to your client machine.
Someone who is more familiar with the code paths that
mosaic follows may be able to tell us that there's no real
threat here, since the file names that get handed to system()
are all internally generated filenames in /tmp, or something
like that. It appears that the bulk of this stuff is in the
HTProtocol switch, where it decodes the URL. The good news
is that everything goes through HTParse first libwww2/HTParse.c
and hopefully some attempt can be made to clean things up there.
For now, though, this is certainly not comforting.
mjr.
Follow-Ups:
|
|
