I noticed this when upgrading to qa0327
If the file /tmp/upgrade/log already exists, an upgrade will overwrite it.
Unless anaconda checks to see if the file's a link, there's the usual
potential for miscreants to exploit that fact to overwrite /etc/shadow or
Hmm, come to think of it, anaconda is also designed to be runnable on the
command line in multi-user mode, so even stat'ing the file before
overwriting it isn't enough to be safe....
Matt would you please add appropriate checks as necessary.
anconda should never be run in multi user mode, I've unlinked the file first now.
You should move the existing file instead (since other products besides RH write
an upgrade.log file) or else give the RH upgrade.log a unique name
(upgrade.log-RH-yearmonthdayhourminutesecond or whatever).
Upgrading to 7.1 whacked an upgrade log from a commercial bioinformatics app
that was kicking around /tmp on one of my machines....
we just remove the file if it is there, which is what we've always done.
I'm reopening this since appropriate checks *still* are not being done, a year
You can do the following to reproduce the security problem:
log on as a normal user to any version of Red Hat
$ cd /tmp ; ln -s ../etc/shadow upgrade.log
and then upgrade the system to Red Hat 7.2 and reboot. When the upgrade
finishes, good luck logging in ;-). /etc/shadow on the upgraded system will
contain the contents of upgrade.log.
Any miscreant can create the link, and just wait for the administrator to
upgrade the system. The same is also true for /tmp/install.log, though that
security hole is far less likely to be exploited in the wild (since /tmp is
typically mkfs'ed when doing an install).
Have you actually tried this with RHL 7.2?
Yep, did it earlier tonight.
I just looked at the code. I don't do much python, but what I see is (this is
from 7.2-7 source):
logname = '/tmp/upgrade.log'
logname = '/tmp/install.log'
instLogName = instPath + logname
instLog = open(instLogName, "w+")
so, it tries to remove the file and then to open it for writing. Seems sane
enough, unless the iutil.rmrf dereferences symlinks.
def rmrf (path):
# this is only the very simple case.
files = os.listdir (path)
for file in files:
if os.path.isdir(path + '/' + file):
rmrf (path + '/' + file)
os.unlink (path + '/' + file)
is maybe os.unlink removing the reference of the symlink rather than the symlink
itself (ie, in this case removing /etc/shadow and leaving the symlink behind,
which it then happily opens for writing, creating a new /etc/shadow)?
No, os.unlink works just fine --
[katzj@mordor tmp]$ ln -s ./blah baz
[katzj@mordor tmp]$ ls -l blah baz
lrwxrwxrwx 1 katzj katzj 6 Mar 20 11:19 baz -> ./blah
-rw-rw-r-- 1 katzj katzj 0 Mar 20 11:14 blah
[root@mordor tmp]# python
Python 1.5.2 (#1, Jul 5 2001, 03:02:19) [GCC 2.96 20000731 (Red Hat Linux 7.1
2 on linux-i386
Copyright 1991-1995 Stichting Mathematisch Centrum, Amsterdam
>>> import os
+ Stopped python
[root@mordor tmp]# ls -l /tmp/bar/
[root@mordor tmp]# ls -l /tmp/blah
-rw-rw-r-- 1 katzj katzj 0 Mar 20 11:14 /tmp/blah
Yeah, so I noticed when playing around.
I can't see how this is happening then. The only other place upgrade.log /
install.log get written is in the dump-to-floppy stuff (exceptions.py). That
does absolutely no sanity checking at all before it writes either of those
files, but so far I can't find any calls to it which could be occurring here....
I'm actually running a test now with 7.2, although I seem to remember doing so
during the last beta cycle and having it work as expected.
Hmm, for some reason when I commented it also ripped off the Cc and changed the
version #, so I'm changing them back. One of these years I'll get the hang of
When I tested it, I did an upgrade from RH 7.2 to RH 7.2 over the network using
a standard bootnet floppy and NFS text install.
This afternoon I'll run a RH 7.1 -> RH 7.2 upgrade just to make sure that no
weirdness was introduced by doing a same version upgrade (shouldn't behave any
differently AFAICT, but...).
Just got it to happen and I see why it's happening; basically, iutil.rmrf
doesn't work if you pass it anything other than a directory. Testing the fix...
And fixed in CVS... we're also going to move them to /root, just as an added