CVE-2007-4559 Patch #2
Conversation
| for member in tar.getmembers(): | ||
| member_path = os.path.join(path, member.name) | ||
| if not is_within_directory(path, member_path): | ||
| raise Exception("Attempted Path Traversal in Tar File") |
There was a problem hiding this comment.
Do not raise Exception but tarfile.TarError (or a derived error): https://docs.python.org/3/library/tarfile.html#tarfile.TarError
| with tarfile.open(str(sdist_tarbal_path), 'r') as sdist_tarbal: | ||
| sdist_tarbal.extractall(temp_dir) | ||
|
|
||
| import os |
There was a problem hiding this comment.
you should not import inside function. Move to module level
|
|
||
| import os | ||
|
|
||
| def is_within_directory(directory, target): |
There was a problem hiding this comment.
these helper functions should go to some helper module so that these could be reused more easily
|
Thanks for the PR. The code using tarfile.extractall is test code that only receives tarfiles created by python setup.py / python -m build during the tests. You are correct that the code does not verify that the extracted tar files are safe but as all of those are created in the tests this isn't that big of on issue. Please run tests and static analyzers using tox (sorry that there is no CI (yet)). Pylint reports the following issues: |
| abs_directory = os.path.abspath(directory) | ||
| abs_target = os.path.abspath(target) | ||
|
|
||
| prefix = os.path.commonprefix([abs_directory, abs_target]) |
There was a problem hiding this comment.
I think this does not work in all cases.
If the target directory is /tmp/a and the tarfile contains path /tmp/ab then os.path.commonprefix(['/tmp/a', '/tmp/ab']) returns '/tmp/a but the file will be extracted to /tmp/ab, i.e. outside the target directory.
You should use os.path.commonpath here (https://docs.python.org/3/library/os.path.html#os.path.commonpath)
2 participants
Patching CVE-2007-4559
Hi, we are security researchers from the Advanced Research Center at Trellix. We have began a campaign to patch a widespread bug named CVE-2007-4559. CVE-2007-4559 is a 15 year old bug in the Python tarfile package. By using extract() or extractall() on a tarfile object without sanitizing input, a maliciously crafted .tar file could perform a directory path traversal attack. We found at least one unsantized extractall() in your codebase and are providing a patch for you via pull request. The patch essentially checks to see if all tarfile members will be extracted safely and throws an exception otherwise. We encourage you to use this patch or your own solution to secure against CVE-2007-4559. Further technical information about the vulnerability can be found in this blog.
If you have further questions you may contact us through this projects lead researcher Kasimir Schulz.