Skip to content

[Security] shutil unpack_archive docs should clarify the security implications #91783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
juaristi opened this issue Apr 21, 2022 · 2 comments · Fixed by #91844
Closed

[Security] shutil unpack_archive docs should clarify the security implications #91783

juaristi opened this issue Apr 21, 2022 · 2 comments · Fixed by #91844
Labels
docs Documentation in the Doc dir

Comments

@juaristi
Copy link

Documentation

The tarfile module is vulnerable to Tar Slip and various other symlink-related attacks. These are known issues that are currently being discussed in the community (see: #65308, #73974).

While tarfile docs show prominent red boxes entitling developers to be careful with tarballs coming from untrusted sources, shutil unpack_archive says nothing about it. However, unpack_archive will leverage tarfile behind the scenes if it sees a .tar.gz (or similar) extension, hence causing unpack_archive inherit all tarfile's security issues.

On the other hand, zipfile is reasonably well protected against these problems. But I believe it's easy for developers to misuse unpack_archive under the assumption that it is equivalent to zipfile, and hence use it without properly sanitizing the input files. And as long as the application only receives zip files there will be no problem, but as soon as a malicious tar file is received, the vulnerabilities are triggered. The problem is that zipfile only accepts zip files, throwing an exception otherwise. But unpack_archive accepts many other formats as well as zip.

@juaristi juaristi added the docs Documentation in the Doc dir label Apr 21, 2022
@dignissimus
Copy link
Contributor

Linking documentation for future reference. I wonder if these documentation changes should be back-ported?

Documentation

tarfile: https://docs.python.org/3/library/tarfile.html#tarfile.TarFile.extract
zipfile: https://docs.python.org/3/library/zipfile.html#zipfile.ZipFile.extract

@JelleZijlstra
Copy link
Member

We should backport docs improvements to at least all the bugfix branches. Since this is a security warning, we should even consider backporting to the security-only branches (3.7 and 3.8).

miss-islington added a commit that referenced this issue May 2, 2022
miss-islington added a commit that referenced this issue May 2, 2022
hello-adam pushed a commit to hello-adam/cpython that referenced this issue Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants