[issue21109] tarfile: Traversal attack vulnerability

STINNER Victor added the comment: What is the status of this issue? -- nosy: +vstinner ___ Python tracker ___ ___ Python-bugs-list

[issue21109] tarfile: Traversal attack vulnerability

Ashwin Ramaswami added the comment: SafeTarFile does not pass the existing tests, mainly because the existing file Lib/test/tarfiletestdata/testtar.tar seems to be "unsafe", producing errors like these: tarfile.SecurityError: : block device tarfile.SecurityError: : duplicate name It

[issue21109] tarfile: Traversal attack vulnerability

Ashwin Ramaswami added the comment: I've added a PR in which I'm working on adding in the tests. Wanted to make sure this is the approach you had in mind? It wasn't as simple as how tests are handled in, say, test_binascii.py, because over there there was only one class that handled the

[issue21109] tarfile: Traversal attack vulnerability

Change by Ashwin Ramaswami : -- pull_requests: +14965 stage: test needed -> patch review pull_request: https://github.com/python/cpython/pull/15244 ___ Python tracker ___

[issue21109] tarfile: Traversal attack vulnerability

Change by Jeffrey Kintscher : -- nosy: +Jeffrey.Kintscher ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue21109] tarfile: Traversal attack vulnerability

Tal Einat added the comment: > Is there any update on this? Will this be fixed in the next release? There was progress made as described on this issue, but there is yet work to be done, and no-one seems to be taking this upon themselves at the moment. I agree that it would be great to have

[issue21109] tarfile: Traversal attack vulnerability

uhei3nn9 added the comment: Is there any update on this? Will this be fixed in the next release? Having a code execution vulnerability (yes it is!) in python for 5 years does not really spark confidence... -- nosy: +uhei3nn9 ___ Python tracker

[issue21109] tarfile: Traversal attack vulnerability

Christian Heimes added the comment: There is some new research on the topic, see https://snyk.io/research/zip-slip-vulnerability, https://github.com/snyk/zip-slip-vulnerability/issues/4#issuecomment-395848367 and BPO #35909 -- ___ Python tracker

[issue21109] tarfile: Traversal attack vulnerability

Tal Einat added the comment: > Having said that, I am refactoring the tests right now since the test file is > ~3000 lines and adding SafeTarFile tests for every TarFile test is cluttering > it. This must be done without adding much test code and with minimal changes to the existing tests.

[issue21109] tarfile: Traversal attack vulnerability

shashank added the comment: It won't exactly be drop-in replacement. I mean if users decide to replace Tarfile with SafeTarFile, existing code may break since there might be cases where dodgy tarballs are acceptable and/or used then SafeTarFile.open will throw an exception. Having said

[issue21109] tarfile: Traversal attack vulnerability

Change by R. David Murray : -- nosy: -r.david.murray ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue21109] tarfile: Traversal attack vulnerability

Tal Einat added the comment: shashank, you're making good progress on this! The tests should also put SafeTarFile through all of the tests for TarFile, considering that it is being described as a drop-in replacement. You should look through the existing tests for other modules which do

[issue21109] tarfile: Traversal attack vulnerability

shashank added the comment: Added tests. Patch file: safetarfile-4.diff Following works with 456 tests passed, after doing `make clean && make` # ./python -m unittest -v test.test_tarfile Attached patch is on top of master's commit: commit 2aaf98c16ae3070378de523a173e29644037d8bd (HEAD ->

[issue21109] tarfile: Traversal attack vulnerability

Tal Einat added the comment: I am not a lawyer, but to the best of my understanding, using such tarballs would be fine. Since Jakub's repo only provides code to generate archive files but doesn't include actual archive files, and the generation code is licensed via the MIT license, we are

[issue21109] tarfile: Traversal attack vulnerability

shashank added the comment: I can't use Jakub's repo (or Makefile from that repo) directly because it relies on tar, which doesn't look like dependency for building Python. I can make similar tarballs but I am not sure how licensing will work. I can add tarballs for the cases I discovered.

[issue21109] tarfile: Traversal attack vulnerability

Tal Einat added the comment: For one thing, the new diffs are still missing tests. Tests should include, at the least: 1. *Safely* testing SafeTarFile against examples of problematic tarballs. Perhaps from Jakub's collection of "sly" tarballs could be used, assuming those could be licensed

[issue21109] tarfile: Traversal attack vulnerability

shashank added the comment: Figured a fix for the bug I found, trick was to keep track of current working dir of symlink it was trying to evaluate. Attached patch: safetarfile-3.diff Patch is for code only. I'd like to see this go thorough, and would appreciate feedback. -- Added

[issue21109] tarfile: Traversal attack vulnerability

Change by STINNER Victor : -- nosy: -vstinner ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue21109] tarfile: Traversal attack vulnerability

shashank added the comment: 1. I have done some changes to Lar's patch to address class of bugs which Jakub found. Attached patch safetarfile-2.diff Patch is for code only and is work in progress. 2. However, there maybe several edge cases which have not been covered. Going by types of

[issue21109] tarfile: Traversal attack vulnerability

shashank added the comment: A. Regrading Jakub's tests, I suppose the changes needed are to for every name in tar i) find reasonable occurrence of symlink's name and replace it with smylink's linkname ii) convert it to normal path and then check for relative / absolute paths B. Jakub, are

[issue21109] tarfile: Traversal attack vulnerability

Jakub Wilk added the comment: I've tested Lars's patch against my collection of sly tarballs: https://github.com/jwilk/path-traversal-samples SafeTarFile defeated most, but not all attacks. It still allows directory traversal for these two tarfile: 1)

[issue21109] tarfile: Traversal attack vulnerability

Philippe Godbout added the comment: Lars, I think the suggested approach is great. Documentation for the tarfile class should be changed in order to direct user to the "safe" version with an relevant warning. A bit like what is done for PRNG safety. As stated by Eduardo an optional "safe"

[issue21109] tarfile: Traversal attack vulnerability

Tal Einat added the comment: Lars, a huge +1 from me for your suggested approach and patch. I'd like to work this into a review-ready PR. The patch is a great step forward but still a ways from being ready for a PR: It is missing tests entirely and there are still several important methods

[issue21109] tarfile: Traversal attack vulnerability

Martin Panter added the comment: Issue 29788 proposes an option to disable the vulnerability in the CLI -- dependencies: +tarfile: Add absolute_path option to tarfile, disabled by default ___ Python tracker

[issue21109] tarfile: Traversal attack vulnerability

Changes by Larry Hastings : -- nosy: -larry ___ Python tracker ___ ___ Python-bugs-list

[issue21109] tarfile: Traversal attack vulnerability

Martin Panter added the comment: Issue 17102 is open about the specific problem of escaping the destination directory. Maybe it is a duplicate, but this bug also discusses other problems. -- dependencies: +tarfile extract can write files outside the destination path

[issue21109] tarfile: Traversal attack vulnerability

Changes by Christian Heimes : -- priority: normal -> high versions: +Python 3.6, Python 3.7 -Python 3.5 ___ Python tracker ___

[issue21109] tarfile: Traversal attack vulnerability

Changes by Jesús Cea Avión j...@jcea.es: -- nosy: +jcea ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21109 ___ ___ Python-bugs-list mailing list

[issue21109] tarfile: Traversal attack vulnerability

Lars Gustäbel added the comment: Let me present for discussion a proposal (and a patch with documentation) with an approach that is a little different, but in my opinion the most effective. I hope that it will appeal to all involved. My proposal consists of a new class SafeTarFile, that is a

[issue21109] tarfile: Traversal attack vulnerability

Eduardo Robles Elvira added the comment: Do we have any final decision on what's the best approach to solve this? I see some possibilities: a) leave the issue to the library user. I think that's a not good solution security-wise as many will be unaware of the problem and this promotes code

[issue21109] tarfile: Traversal attack vulnerability

Eduardo Robles Elvira added the comment: Also, I guess this patch solves and is closely related to #1044 which was, at the time (2007), considered not a bug. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21109

[issue21109] tarfile: Traversal attack vulnerability

Changes by Jakub Wilk jw...@jwilk.net: -- nosy: +jwilk ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21109 ___ ___ Python-bugs-list mailing list

[issue21109] tarfile: Traversal attack vulnerability

Martin Panter added the comment: Seems like shutil._unpack_tarfile() is affected. I guess it could at least do with one of those warnings in the documentation for make_archive(). The patch for this bug looks a bit over enthusiastic, for example skip_prefixes(blaua../stuff) would incorrectly

[issue21109] tarfile: Traversal attack vulnerability

Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com: -- nosy: +Arfrever ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21109 ___

[issue21109] tarfile: Traversal attack vulnerability

Lars Gustäbel added the comment: In the past, our answer to these kinds of bug reports has always been that you must not extract an archive from an untrusted source without making sure that it has no malicious contents. And that tarfile conforms to the posix specifications with respect to

[issue21109] tarfile: Traversal attack vulnerability

Larry Hastings added the comment: Thank you Lars for your thorough reply. While I agree that this isn't a release blocker, as it was clearly designed to behave this way... it seems to me that it wouldn't take much to make the tarfile module a lot safer. Specifically: * Don't allow

[issue21109] tarfile: Traversal attack vulnerability

New submission from Daniel Garcia: The application does not validate the filenames inside the tar archive, allowing to extract files in arbitrary path. An attacker can craft a tar file to override files. I've view this vulnerability in libtar: http://lwn.net/Vulnerabilities/587141/ I've

[issue21109] tarfile: Traversal attack vulnerability

Daniel Garcia added the comment: The solution in the patch is based on the gnutar solution to this, removing the prefix when extracting and adding. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21109

[issue21109] tarfile: Traversal attack vulnerability

Ned Deily added the comment: Setting as release blocker pending evaluation. -- keywords: +security_issue nosy: +benjamin.peterson, georg.brandl, larry, lars.gustaebel, ned.deily priority: normal - release blocker stage: - test needed versions: +Python 2.7, Python 3.1, Python 3.2,

[issue21109] tarfile: Traversal attack vulnerability

Christian Heimes added the comment: It's a known and well-documented behavior of the tar module: https://docs.python.org/2.7/library/tarfile.html#tarfile.TarFile.extractall -- nosy: +christian.heimes ___ Python tracker rep...@bugs.python.org

[issue21109] tarfile: Traversal attack vulnerability

STINNER Victor added the comment: It's a known and well-documented behavior of the tar module Would it possible to disable this behaviour by default, and only enable ti explicitly? The tar command line program has for example the -P / --absolute-paths option. -- nosy: +haypo

[issue21109] tarfile: Traversal attack vulnerability

Serhiy Storchaka added the comment: Yes, this behavior is documented, but still it is desirable to fix it. The tar utility has a lot of switches which controls extracting and by default it prevents three ways of attack (absolute names, '..' and symlinks), but there are other possible ways of

[issue21109] tarfile: Traversal attack vulnerability

R. David Murray added the comment: Note that any issues here should also be considered for zipfile and shutil. (Well, shutil can just use the other two once the security is available.) See issue 20907. -- nosy: +r.david.murray ___ Python tracker

[issue21109] tarfile: Traversal attack vulnerability

Christian Heimes added the comment: Don't forget about SUID and SGID, too. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21109 ___ ___