Bug 34030 - upgrade overwrites existing files in /tmp
upgrade overwrites existing files in /tmp
Status: CLOSED RAWHIDE
Product: Red Hat Linux
Classification: Retired
Component: anaconda (Show other bugs)
7.2
i386 Linux
high Severity medium
: ---
: ---
Assigned To: Matt Wilson
Brock Organ
: Security
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2001-03-30 02:57 EST by Chris Ricker
Modified: 2007-03-26 23:43 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2002-03-21 12:16:52 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Chris Ricker 2001-03-30 02:57:31 EST
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....
Comment 1 Michael Fulbright 2001-03-30 10:16:59 EST
Matt would you please add appropriate checks as necessary.
Comment 2 Matt Wilson 2001-03-30 17:57:43 EST
anconda should never be run in multi user mode, I've unlinked the file first now.
Comment 3 Chris Ricker 2001-04-18 09:05:35 EDT
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....
Comment 4 Matt Wilson 2001-07-20 17:00:14 EDT
we just remove the file if it is there, which is what we've always done.
Comment 5 Chris Ricker 2002-03-19 23:19:43 EST
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).
Comment 6 Matt Wilson 2002-03-19 23:27:03 EST
Have you actually tried this with RHL 7.2?
Comment 7 Chris Ricker 2002-03-19 23:31:34 EST
Yep, did it earlier tonight.
Comment 8 Chris Ricker 2002-03-19 23:59:26 EST
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)?
Comment 9 Jeremy Katz 2002-03-20 11:27:13 EST
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
Comment 10 Chris Ricker 2002-03-20 11:58:51 EST
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....
Comment 11 Jeremy Katz 2002-03-20 12:03:44 EST
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.
Comment 12 Chris Ricker 2002-03-20 12:09:41 EST
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...).
Comment 13 Jeremy Katz 2002-03-20 12:17:53 EST
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...
Comment 14 Jeremy Katz 2002-03-20 18:47:03 EST
And fixed in CVS...  we're also going to move them to /root, just as an added
precaution

Note You need to log in before you can comment on or make changes to this bug.