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

29 Apr 2011

      On Fri, Apr 29, 2011 at 10:02 AM, Eli Bendersky  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  wrote:
...
On Thu, 28 Apr 2011 17:40:05 +0200
Nadeem Vawda  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