homepage

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: zipfile: fix arcname with leading '///' or '..'
Type: security Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: duplicate
Dependencies: Superseder: zipfile.ZipFile overwrites files outside destination path
View: 6972
Assigned To: Nosy List: amaury.forgeotdarc, serhiy.storchaka, zhigang
Priority: normal Keywords: patch

Created on 2011年01月14日 12:03 by zhigang, last changed 2022年04月11日 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
python-zipfile-fix-arcname.patch zhigang, 2011年01月14日 12:03
Messages (6)
msg126254 - (view) Author: Zhigang Wang (zhigang) Date: 2011年01月14日 12:03
We only support arcname with one leading '/', but not more. This patch fixes it.
We don't support arcname with '..' well. The default behavior of unzip and 7z is to ignore all '..'. This patch does the same.
Also updated the doc. If there are other security related issues exist, we should revise the doc.
Please review.
msg126255 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011年01月14日 12:20
What happens when the archive contains both 'foo' and '../foo'? They seem to be extracted at the same place.
msg126258 - (view) Author: Zhigang Wang (zhigang) Date: 2011年01月14日 13:20
$ unzip -l t.zip 
Archive: t.zip
 Length Date Time Name
--------- ---------- ----- ----
 3 01-14-2011 21:11 ../foo
 3 01-14-2011 21:11 foo
--------- -------
 6 2 files
[zhigang@localhost tmp]$ unzip -d aa t.zip
Archive: t.zip
warning: skipped "../" path component(s) in ../foo
 extracting: aa/foo 
replace aa/foo? [y]es, [n]o, [A]ll, [N]one, [r]ename: A
 extracting: aa/foo 
$ 7za x -oaa t.zip 
7-Zip (A) 9.13 beta Copyright (c) 1999-2010 Igor Pavlov 2010年04月15日
p7zip Version 9.13 (locale=en_US.UTF-8,Utf16=on,HugeFiles=on,1 CPU)
Processing archive: t.zip
file aa/foo
already exists. Overwrite with 
../foo?
(Y)es / (N)o / (A)lways / (S)kip all / A(u)to rename all / (Q)uit? A
Extracting ../foo
Extracting foo
Everything is Ok
Files: 2
Size: 6
Compressed: 198
msg126259 - (view) Author: Zhigang Wang (zhigang) Date: 2011年01月14日 13:29
Yes, in zipfile, we just overwrite it. Actually, ZipFile.extract() overwrite existing files already. If we want it more powerful, we can add a 'overwrite' parameter. But turning zipfile a full featured zip/unzip tool needs much more extra work...
msg173478 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012年10月21日 20:16
Some comments to patch.
+ arcname = os.path.sep.join([x for x in arcname.split(os.path.sep)
+ if x != '..'])
File names in zip archive should use '/' as separator, not os.path.sep. '../spam' will be not cleaned by this code.
+ while arcname[0] in (os.sep, os.altsep):
+ arcname = arcname[1:]
It will not save from filenames containing drive letter: 'C:/Windows/python.exe'.
msg173525 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012年10月22日 14:44
I'm going to close this issue as a duplicate of issue6972. Issue6972 is older and has a larger discussion.
Thank you for patch and research, Zhigang Wang. I will use it for the new patch.
History
Date User Action Args
2022年04月11日 14:57:11adminsetgithub: 55114
2013年01月26日 18:16:31serhiy.storchakasetstatus: pending -> closed
stage: resolved
2012年10月22日 14:44:47serhiy.storchakasetstatus: open -> pending
superseder: zipfile.ZipFile overwrites files outside destination path
resolution: duplicate
messages: + msg173525
2012年10月21日 20:16:28serhiy.storchakasetmessages: + msg173478
2012年04月07日 19:17:25serhiy.storchakasetnosy: + serhiy.storchaka
2011年01月14日 13:29:47zhigangsetmessages: + msg126259
2011年01月14日 13:20:50zhigangsetmessages: + msg126258
2011年01月14日 12:20:30amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg126255
2011年01月14日 12:03:45zhigangcreate

AltStyle によって変換されたページ (->オリジナル) /