31

I'm trying to extract user-submitted zip and tar files to a directory. The documentation for zipfile's extractall method (similarly with tarfile's extractall) states that it's possible for paths to be absolute or contain .. paths that go outside the destination path. Instead, I could use extract myself, like this:

some_path = '/destination/path'
some_zip = '/some/file.zip'
zipf = zipfile.ZipFile(some_zip, mode='r')
for subfile in zipf.namelist():
    zipf.extract(subfile, some_path)

Is this safe? Is it possible for a file in the archive to wind up outside of some_path in this case? If so, what way can I ensure that files will never wind up outside the destination directory?

1
  • 1
    Starting with python 2.7.4, the method zipfile.extract() prohibits the creation of files outside the sandbox. So, this method is now safe as of python 2.7.4. The vulnerability still exists for tar archives, however.
    – alexis
    Commented Jun 7, 2013 at 16:32

5 Answers 5

45
+100

Note: Starting with python 2.7.4, this is a non-issue for ZIP archives. Details at the bottom of the answer. This answer focuses on tar archives.

To figure out where a path really points to, use os.path.abspath() (but note the caveat about symlinks as path components). If you normalize a path from your zipfile with abspath and it does not contain the current directory as a prefix, it's pointing outside it.

But you also need to check the value of any symlink extracted from your archive (both tarfiles and unix zipfiles can store symlinks). This is important if you are worried about a proverbial "malicious user" that would intentionally bypass your security, rather than an application that simply installs itself in system libraries.

That's the aforementioned caveat: abspath will be misled if your sandbox already contains a symlink that points to a directory. Even a symlink that points within the sandbox can be dangerous: The symlink sandbox/subdir/foo -> .. points to sandbox, so the path sandbox/subdir/foo/../.bashrc should be disallowed. The easiest way to do so is to wait until the previous files have been extracted and use os.path.realpath(). Fortunately extractall() accepts a generator, so this is easy to do.

Since you ask for code, here's a bit that explicates the algorithm. It prohibits not only the extraction of files to locations outside the sandbox (which is what was requested), but also the creation of links inside the sandbox that point to locations outside the sandbox. I'm curious to hear if anyone can sneak any stray files or links past it.

import tarfile
from os.path import abspath, realpath, dirname, join as joinpath
from sys import stderr

resolved = lambda x: realpath(abspath(x))

def badpath(path, base):
    # joinpath will ignore base if path is absolute
    return not resolved(joinpath(base,path)).startswith(base)

def badlink(info, base):
    # Links are interpreted relative to the directory containing the link
    tip = resolved(joinpath(base, dirname(info.name)))
    return badpath(info.linkname, base=tip)

def safemembers(members):
    base = resolved(".")
    
    for finfo in members:
        if badpath(finfo.name, base):
            print >>stderr, finfo.name, "is blocked (illegal path)"
        elif finfo.issym() and badlink(finfo,base):
            print >>stderr, finfo.name, "is blocked: Symlink to", finfo.linkname
        elif finfo.islnk() and badlink(finfo,base):
            print >>stderr, finfo.name, "is blocked: Hard link to", finfo.linkname
        else:
            yield finfo

ar = tarfile.open("testtar.tar")
ar.extractall(path="./sandbox", members=safemembers(ar))
ar.close()

Edit: Starting with python 2.7.4, this is a non-issue for ZIP archives: The method zipfile.extract() prohibits the creation of files outside the sandbox:

Note: If a member filename is an absolute path, a drive/UNC sharepoint and leading (back)slashes will be stripped, e.g.: ///foo/bar becomes foo/bar on Unix, and C:\foo\bar becomes foo\bar on Windows. And all ".." components in a member filename will be removed, e.g.: ../../foo../../ba..r becomes foo../ba..r. On Windows, illegal characters (:, <, >, |, ", ?, and *) [are] replaced by underscore (_).

The tarfile class has not been similarly sanitized, so the above answer still apllies.

9
  • You can assume the new sandbox directory is empty
    – jterrace
    Commented Apr 11, 2012 at 16:30
  • I thought as much; but you still need to watch out for the exploit I outlined: First the archive contains a symlink to another directory, then a file that uses the symlink as its path.
    – alexis
    Commented Apr 11, 2012 at 16:52
  • 1
    realpath will convert the extracted file into its real path, so you could probably just check that after extraction?
    – jterrace
    Commented Apr 11, 2012 at 17:05
  • Right, you can use realpath to test every symlink immediately after extracting it (which means you can't use extractall to unzip the archive, since you need to check after extracting each file).
    – alexis
    Commented Apr 11, 2012 at 17:18
  • 1
    According to the readme, Archive.extract() will raise an exception if it detects an out-of-bounds file. The exception will terminate the bulk extraction, and there's no way to resume it. There doesn't even seem to be a way to list the archive contents and extract one file at a time. Color me unimpressed.
    – alexis
    Commented May 20, 2015 at 20:46
4

Contrary to the popular answer, unzipping files safely is not completely solved as of Python 2.7.4. The extractall method is still dangerous and can lead to path traversal, either directly or through the unzipping of symbolic links. Here was my final solution which should prevent both attacks in all versions of Python, even versions prior to Python 2.7.4 where the extract method was vulnerable:

import zipfile, os

def safe_unzip(zip_file, extract_path='.'):
    with zipfile.ZipFile(zip_file, 'r') as zf:
        for member in zf.infolist():
            file_path = os.path.realpath(os.path.join(extract_path, member.filename))
            if file_path.startswith(os.path.realpath(extract_path)):
                zf.extract(member, extract_path)

Edit 1: Fixed variable name clash. Thanks Juuso Ohtonen.

Edit 2: s/abspath/realpath/g. Thanks TheLizzard

4
  • 2
    Avoid using zipfile as parameter name since it conflicts the import name: AttributeError: 'str' object has no attribute 'ZipFile'. Fix is to rename zipfile parameter to e.g. zip_file. Commented Oct 6, 2016 at 11:31
  • Thanks, for the comment. I fixed the sample code. I originally grabbed it out of my project and edited it to be stand-alone and clearly forgot to test it.
    – shellster
    Commented Oct 7, 2016 at 17:01
  • 2
    Why are you using os.path.abspath not os.path.realpath? Wouldn't it be more safe to use os.path.realpath?
    – TheLizzard
    Commented Dec 18, 2020 at 9:59
  • Good point. I'll update the answer to reflect that suggestion. realpath apparently calls abspath, so realpath should be sufficient.
    – shellster
    Commented Dec 18, 2020 at 14:29
3

Use ZipFile.infolist()/TarFile.next()/TarFile.getmembers() to get the information about each entry in the archive, normalize the path, open the file yourself, use ZipFile.open()/TarFile.extractfile() to get a file-like for the entry, and copy the entry data yourself.

6
  • 4
    This seems really tricky to make sure I get right - especially if you have files like ../../../../subdir/../../something/file.txt - where should the destination be? No one has made code available to deal with this before?
    – jterrace
    Commented Apr 8, 2012 at 3:22
  • 3
    No one can answer that for you, since only you understand your application requirements. Commented Apr 8, 2012 at 3:25
  • 4
    I disagree. Other tools do this automatically for you - for example the tar command automatically gets rid of absolute paths unless you specify --absolute-names.
    – jterrace
    Commented Apr 8, 2012 at 3:30
  • 1
    And any software that delegates to tar has to abide by that. This is your software. Commented Apr 8, 2012 at 3:38
  • 7
    sigh When you come across an entry with an invalid/disallowed path you have 3 options: 1) attempt extraction anyway, and catch any errors 2) extract to a modified path 3) don't extract. I can't tell you which policy is appropriate for your application. Commented Apr 8, 2012 at 4:02
3

Copy the zipfile to an empty directory. Then use os.chroot to make that directory the root directory. Then unzip there.

Alternatively, you can call unzip itself with the -j flag, which ignores the directories:

import subprocess
filename = '/some/file.zip'
rv = subprocess.call(['unzip', '-j', filename])
4
  • The subprocess module works on every platform that runs Python, AFAICT. But if you are talking about MS Windows, There are several programs for handling zipfiles available for it, like INFO-zip. The specific command line would of course need to be adapted for the program you wish to use. Commented Apr 16, 2012 at 23:29
  • You're right, os.chroot is specific to UNIX. But if you search for them you'll find chroot like applications for windows. Of course the real overkill solution in this case would be to run unzip in a virtual machine. :-) Commented Apr 19, 2012 at 21:29
  • 2
    That's a brilliantly simple idea, but (a) it only really works on Unix systems, and (b) on Unix, only the superuser can chroot. Privilege escalation in the midst of dealing with potentially unsafe data is really the wrong way to go...
    – alexis
    Commented May 7, 2012 at 10:43
  • Using the -j flag of info-zip's unzip as an alternative of chroot should work on any platform that unzip works on. Commented May 9, 2012 at 12:45
2

PSA: The accepted answer to this question is out of date!

As of Python release 3.11.4 there is an extraction filter mechanism included in tarfile.TarFile.extractall(). This mechanism, when using the data filter, will ensure safe extraction of tarballs in most cases (including CVE-2007-4559).

If you have the ability, you should use a version of python >=3.11.4 when processing untrusted tar files so as to avail yourself of the provided security features. The accepted answer should be implemented iff you can't use the language feature for this purpose.

fair thee well fellow exhausted engineer...

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.