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 similar. 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 later. 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): (packages.py) if upgrade: logname = '/tmp/upgrade.log' else: logname = '/tmp/install.log' instLogName = instPath + logname try: iutil.rmrf (instLogName) except OSError: pass 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. (iutil.py) 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) else: os.unlink (path + '/' + file) os.rmdir (path) 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 >>> os.unlink("/tmp/baz") >>> [1]+ Stopped python [root@mordor tmp]# ls -l /tmp/bar/ total 0 [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 bugzilla ;-) 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 precaution