[Python-Dev] [Python-checkins] cpython (2.7): Fix closes issue10761: tarfile.extractall failure when symlinked files are (original) (raw)

Nadeem Vawda nadeem.vawda at gmail.com
Fri Apr 29 11:24:48 CEST 2011

On Fri, Apr 29, 2011 at 10:02 AM, Eli Bendersky <eliben at gmail.com> wrote:

I completely understand this "other code/thread deletes the path between exists() and unlink()" case - it indeed is a race condition waiting to happen. What I didn't understand was Antoine's example of "attacker creates targetpath between os.path.exists and os.unlink", and was asking for a more detailed example, since I'm not really experienced with security-oriented thinking.

If targetpath is created after the os.path.exists() check, then os.unlink() will not be called, so os.symlink() will raise an exception when it sees that targetpath already exists.

On Thu, Apr 28, 2011 at 5:44 PM, Antoine Pitrou <solipsis at pitrou.net> wrote:

On Thu, 28 Apr 2011 17:40:05 +0200 Nadeem Vawda <nadeem.vawda at gmail.com> wrote:

The deletion case could be handled like this: if tarinfo.issym(): + try: + os.unlink(targetpath) + except OSError as e: + if e.errno != errno.ENOENT: + raise os.symlink(tarinfo.linkname, targetpath) Someone can still create "targetpath" between the calls to unlink() and symlink() though.

Like I said, that code only handles the case of targetpath being deleted. I can't think of a similarly easy fix for the creation case. You could solve that particular form of the problem with something like:

if tarinfo.issym():
    while True:
        try:
            os.symlink(tarinfo.linkname, targetpath)
        except OSError as e:
            if e.errno != errno.EEXIST:
                raise
        else:
            break
        try:
            os.unlink(targetpath)
        except OSError as e:
            if e.errno != errno.ENOENT:
                raise

... but that would open up another DOS vulnerability - if an attacker manages to keep re-creating targetpath in the window between unlink() and symlink(), the loop would never terminate. Also, the code is rather ugly ;-)

Cheers, Nadeem

More information about the Python-Dev mailing list