This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: tarfile extractall() allows local attacker to overwrite files while extracting
Type: security Stage:
Components: Library (Lib) Versions: Python 3.0, Python 2.6, Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: lars.gustaebel Nosy List: alanmcintyre, georg.brandl, lars.gustaebel, loewis, mebrown
Priority: high Keywords:

Created on 2008-02-03 03:59 by mebrown, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
zipfile-dirperm.diff alanmcintyre, 2008-02-03 05:29
tarfile-diffs.tar.gz lars.gustaebel, 2008-02-04 10:51
Messages (9)
msg62016 - (view) Author: Michael Brown (mebrown) Date: 2008-02-03 03:59
python 2.5.1
tarfile.py line 1516 in extractall() 

sets directories created to world-writeable while extracting which means
an attacker can change/modify files before perms are fixed. Suggest 770
while extracting to fix.
msg62017 - (view) Author: Michael Brown (mebrown) Date: 2008-02-03 04:12
I can confirm that this issue has been addressed in trunk tarfile.py.
msg62018 - (view) Author: Alan McIntyre (alanmcintyre) * (Python committer) Date: 2008-02-03 05:28
I noticed that in the trunk, ZipFile._extract_member, at line 865, still
uses 777 (the default of os.makedirs) to create directories.  I attached
a patch for it.

A quick grep shows that tarfile still uses the default permissions for
os.makedirs and mkdir.  Should these all be changed to 700?
msg62021 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-02-03 08:01
Lars, can you take a look?
msg62039 - (view) Author: Lars Gustäbel (lars.gustaebel) * (Python committer) Date: 2008-02-04 10:51
This was fixed in the trunk in r53526 about a year ago following
issue1507247. I did not backport it to 2.5 at that time, because it
would have changed existing behaviour. If there are no objections I
could do this now.

The os.mkdir() call in TarFile.makedir() uses the default mode, but the
real mode from the TarInfo object is applied two instructions later in
TarFile._extract_member(). I have nothing against using 0700 in
TarFile.makedir().

The os.makedirs() call in _extract_member() (trunk) is fine. It creates
missing directories that are not part of the archive with default
permissions, that is mode 0777 with the current umask masked out.

I attached a patchset against the release25-maint branch and the trunk
that is supposed to fix the issue and harmonizes the code between the
two versions.
msg62050 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-02-04 21:25
Even though it does change existing behaviour, can anybody imagine a
case where extracting a tarfile now fails when it previously succeeds?

If not, I would consider this a security-relevant fix, and thus a
candidate for a backport. Perhaps this should be raised with
security@python.org.
msg62064 - (view) Author: Lars Gustäbel (lars.gustaebel) * (Python committer) Date: 2008-02-05 10:56
os.mkdir() and os.makedirs() always apply the current umask to the mode.
We cannot be responsible for poorly chosen umasks. In general, tarfile
and zipfile create directories with reasonable modes. So, IMO the
zipfile-dirperm.diff is not needed and Michael's problem depends on the
current umask.

The only exception is in TarFile._extract_member() in Python <= 2.5.x
that creates missing directories that are not part of the archive(!) and
uses os.chmod() to force a 0777 mode. That problem was addressed in
issue1507247 but only for Python 2.6 and should be backported. Although
this would change behaviour it would not cause failures.
msg62069 - (view) Author: Lars Gustäbel (lars.gustaebel) * (Python committer) Date: 2008-02-05 12:06
I took the liberty of applying my patches to the trunk (r60588) and the
release25-maint branch (r60589).
msg62070 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-02-05 12:07
Closing as fixed.
History
Date User Action Args
2022-04-11 14:56:30adminsetgithub: 46288
2008-02-05 12:07:42georg.brandlsetstatus: open -> closed
nosy: + georg.brandl
resolution: fixed
messages: + msg62070
2008-02-05 12:06:39lars.gustaebelsetmessages: + msg62069
2008-02-05 10:56:45lars.gustaebelsetmessages: + msg62064
2008-02-04 21:25:51loewissetmessages: + msg62050
2008-02-04 10:51:46lars.gustaebelsetfiles: + tarfile-diffs.tar.gz
messages: + msg62039
2008-02-03 19:59:26christian.heimessetpriority: high
versions: + Python 2.6, Python 3.0
2008-02-03 08:01:09loewissetassignee: lars.gustaebel
messages: + msg62021
nosy: + loewis, lars.gustaebel
2008-02-03 05:29:01alanmcintyresetfiles: + zipfile-dirperm.diff
nosy: + alanmcintyre
messages: + msg62018
2008-02-03 04:12:43mebrownsetmessages: + msg62017
2008-02-03 03:59:12mebrowncreate