zipfile can't extract file: file name in directory and header differ

@birkenfeld

BPO	6839
Nosy	@birkenfeld, @terryjreedy, @gpshead, @ronaldoussoren, @amauryfa, @ncoghlan, @berkerpeksag, @JimJJewett, @serhiy-storchaka, @MorganRamsay
PRs	bpo-6839: removed unnecessary file name encoding test from ZipFile.open() #14212
Files	test.zip: mildly corrupt zipfile to test error handling zlib_forward_slash.patch zipfile_276_filename_mismatch_v2.patch: patch with warnings against 2.7.6 zipfile_340_filename_mismatch_v3.patch: patch with warnings against 3.4.0 zipfile_276_filename_mismatch_v3.patch: patch with print against 2.7.6

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2009-09-04.19:56:49.966>
labels = ['3.7', 'type-bug', 'library', 'OS-windows']
title = "zipfile can't extract file"
updated_at = <Date 2019-06-18.21:36:54.889>
user = 'https://bugs.python.org/NewerCookie'

bugs.python.org fields:

activity = <Date 2019-06-18.21:36:54.889>
actor = 'python-dev'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)', 'Windows']
creation = <Date 2009-09-04.19:56:49.966>
creator = 'NewerCookie'
dependencies = []
files = ['14832', '14934', '35115', '35117', '35120']
hgrepos = []
issue_num = 6839
keywords = ['patch']
message_count = 61.0
messages = ['92265', '92297', '92309', '92326', '92330', '92335', '92516', '92874', '116384', '116385', '200165', '201842', '208970', '208973', '208975', '208982', '208983', '208985', '208987', '209023', '211562', '217533', '217546', '217551', '217554', '217556', '217558', '217561', '217569', '217570', '217571', '217572', '217573', '217574', '217575', '217576', '217578', '217616', '217624', '217625', '217627', '217634', '217635', '217636', '217638', '217641', '217642', '217643', '217647', '217648', '217659', '217660', '217661', '217743', '217753', '217754', '217756', '217778', '217932', '217933', '345999']
nosy_count = 15.0
nosy_names = ['georg.brandl', 'terry.reedy', 'gregory.p.smith', 'ronaldoussoren', 'amaury.forgeotdarc', 'ncoghlan', 'NewerCookie', 'chuck', 'francismb', 'berker.peksag', 'Jim.Jewett', 'serhiy.storchaka', 'apolkosnik', 'Sean Goodwin', 'MorganRamsay']
pr_nums = ['14212']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue6839'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6', 'Python 3.7']

The following exception occured when I tried to extract on Windows.

"zipfile.BadZipfile: File name in directory "test\test2.txt" and header
"test/test2.txt" differ."

It seems like problem about slash.
I tested using by zipfile Revision 72893.

P.S
I tested extraction by using 7-zip.
It works fine.

The zipfile is technically incorrect, the zipfile specification prescribes
that all filenames use '/' as the directory separator.

Even without that caveat the file is corrupt because the zipfile directory
header and the per-file header don't agree on the name of the file.

That said: IMHO the current code in zipfile.ZipFile.open is too strict, it
shouldn't raise an error when the two names aren't exactly the same
because there are valid reasons for them to be different (such as renaming
a file in the zipfile without rewriting the entire zipfile).

FileRoller doesn't complain about the mismatched slashes either. Where
did the ZIP come from, by the way? I seem to recall that there have
been other instances in which ZIP applications were more "forgiving"
than the zipfile module. How far should zipfile go in bending the
interpretation of the ZIP standard?

As far as the renaming goes, it seems the standard says the header name
should be used if the two names are different. If nobody else has time
to make a patch and tests I can take a stab at it in the next few days.

alan: I don't quite understand which filename you want to use when the
name in the per-file header and the central directory don't match.

Where in the standard is this prescribed? I couldn't find anything in
the PKWare zipfile appnote [1]

My preference would be to use the central directory as the canonical
value because scanning the entire zipfile to read the per-file header
would give a significant overhead. This might not be very noticable with
small zipfiles, but I regularly use zipfiles with over 100K files in
them in those files a scan of the zipfile is prohibitively expensive.

Furthermore, when the two are different the most reasonably explaination
is that an in-place edit of the zipfile changed the directory without
rewriting the entire zipfile (just like you can "delete" files from a
zipfile by dumping them from the directory rather than completely
rewriting the entire archive)

[1]
APPNOTE.TXT - .ZIP File Format Specification Version: 6.3.2
Revised: September 28, 2007
Copyright (c) 1989 - 2007 PKWARE Inc., All Rights Reserved.

Sorry about the confusion--I think I confused myself by looking at the
bit about CRC checksums in the "Info-ZIP Unicode Path Extra Field"
section before I posted. I meant to say that the central directory name
looks preferred over the per-file header.

n section J, under "file name (Variable)" there's a bit that says:

"If input came from standard input, there is no file name field. If
encrypting the central directory and general purpose bit flag 13 is set
indicating masking, the file name stored in the Local Header will not be
the actual file name. A masking value consisting of a unique
hexadecimal value will be stored."

So in these cases the central directory name has to be used. And, as
you pointed out, some operations like "deleting" a member from the
archive are implemented by editing the central directory, so it would
seem that the central directory should be used if there's a conflict.

In the case at issue, the file name is the same (contrary to the error
message). The two representations of the *path* are different, but
equivalent. There is no ambiguity: the file should be put in directory
'test' and named 'test2.txt'. So I think zipfile should do what 7zip
does and do just that.

An actual filename difference might be argued differently.

I added a patch to replace back slashes by forward slashes in three
places, only one if them actually relevant to the errors in the attached
.zip file.

I kept the exception for mismatching filenames, but if you think it is
appropriate to remove it I could do that as well.

I agree with the change, but the code should be factorized in a function (normalize_filename for example)

I'd prefer if the code no longer checked if the filename in the directory matches the name in the per-file header.

The reason of that is that the two don't have to match: it is relatively cheap to rename a file in the zipfile by rewriting the directory while rewriting the entire zipfile can be pretty expensive when zipfiles get large.

It's probably worthwhile to test what other zipfile tools do in the respect (e.g., create a zipfile where the filename in the header doesn't match the name in the directory and extract that zip using a number of popular tools).

(I have a slightly odd perspective on this because I regularly deal with zipfiles containing over 100K files and over 10GByte of data).

Adam Polkasnik said:
--------------------

Extraction works fine, the issue was that raise() was creating an exception, and
stopping the whole extraction process.

That doesn't make sense. If an exception was "stopping the whole extraction process" then extraction was not working fine.

Questions:

Are the names with '\' in them in the central directory, or the per-file header?
If in the central directory (which is the name we are going to use, yes?) how do
we tell if the '\' should be a '/' or an escape? (such as '\t')

Ethan,
I'd refer you to msg92309...

And
When testing with WinZip it looks like this:
No errors detected in compressed data of C:\Downloads\test.zip.
Testing ...
Testing test\ OK
Testing test\test2.txt OK
Testing test1.txt OK

Then in python:
Python 3.4.0 (v3.4.0:04f714765c13, Mar 16 2014, 19:25:23) [MSC v.1600 64 bit (AM
D64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import zipfile
>>> zf =  zipfile.ZipFile('test.zip')
>>> namelist = zf.namelist()
>>> namelist
['test/', 'test/test2.txt', 'test1.txt']
>>> for af in namelist:
...     zf.read(af)
...
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
  File "c:\Python34\lib\zipfile.py", line 1117, in read
    with self.open(name, "r", pwd) as fp:
  File "c:\Python34\lib\zipfile.py", line 1180, in open
    % (zinfo.orig_filename, fname))
zipfile.BadZipFile: File name in directory 'test\\' and header b'test/' differ.

So, based on that everything is already converted to forward slashes for the extraction.

Ah, so when you (Adam) said "extraction works fine", what you meant was "extraction works fine *in other programs*". Okay.

Both. Other programs, and in python scripts when raise() is removed in zipfile.py. Unless your results are different.

The encoding test in ZipFile.open() is highly opinionated and has no purpose beyond itself. Testing for encoding issues should be done outside this library in the user's own code.

Using the 3.7.2 version of ZipFile, this is my proposal:

https://gist.github.com/MorganRamsay/696e89450e0f172c16ac8dfc016eb79f/revisions?diff=unified

Currently, I'm subclassing ZipFile with this patch and I've had no issues with extracting thousands of different ZIP files on Windows. I can't attest to this solution's applicability on other platforms.

I think the core of this bug is that we don't do the "/"-normalization in

cpython/Lib/zipfile/__init__.py

Lines 1733 to 1742 in 6b73502

    
           if fheader[_FH_GENERAL_PURPOSE_FLAG_BITS] & _MASK_UTF_FILENAME: 
        
               # UTF-8 filename 
        
               fname_str = fname.decode("utf-8") 
        
           else: 
        
               fname_str = fname.decode(self.metadata_encoding or "cp437") 
        
           if fname_str != zinfo.orig_filename: 
        
               raise BadZipFile( 
        
                   'File name in directory %r and header %r differ.' 
        
                   % (zinfo.orig_filename, fname))

Would fixing that be sufficient?

I'm also not sure this looks at the unicode filename extra, and probably should. This is a case where we could refactor them to use the same code...

There is a good reason to keep the exception. This protects us from the simplest kind of zipbombs. If remove it, then the attacker can create a ZIP file containing one large file and many entries with different files in the central directory referring the same local header. Both the size and the number of entries under threshold. Extracting such archive will lead to occupying disk space equal to multiplication of the single file size and the number of entries.

Normalization of only slashes on Windows before the check will protect us.

Normalization of slashes on Posix platforms will be a breaking change. Backslash has no special meaning in the path, it can be a part of normal file name. So this solution will not help on Posix.

There is a good reason to keep the exception. This protects us from the simplest kind of zipbombs.

We have two goals then:

Don't raise a BadZipFile exception for archives with uneven path separators.
Prevent zipbombs.

Path normalization works for Windows users, but not so much for users on other platforms.

What do you think about offset validation?

Navigation Menu

zipfile can't extract file: file name in directory and header differ #51088

46 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

	if fheader[_FH_GENERAL_PURPOSE_FLAG_BITS] & _MASK_UTF_FILENAME:
	# UTF-8 filename
	fname_str = fname.decode("utf-8")
	else:
	fname_str = fname.decode(self.metadata_encoding or "cp437")

	if fname_str != zinfo.orig_filename:
	raise BadZipFile(
	'File name in directory %r and header %r differ.'
	% (zinfo.orig_filename, fname))

zipfile can't extract file: file name in directory and header differ #51088

Description

Activity

NewerCookie commented on Sep 4, 2009

NewerCookie commented on Sep 6, 2009

ronaldoussoren commented on Sep 6, 2009

alanmcintyre commented on Sep 6, 2009

ronaldoussoren commented on Sep 6, 2009

alanmcintyre commented on Sep 6, 2009

terryjreedy commented on Sep 11, 2009

chuck commented on Sep 19, 2009

amauryfa commented on Sep 14, 2010

ronaldoussoren commented on Sep 14, 2010

46 remaining items

ethanfurman commented on May 2, 2014

apolkosnik commented on May 2, 2014

ethanfurman commented on May 5, 2014

apolkosnik commented on May 5, 2014

MorganRamsay commented on Jun 18, 2019

thatch commented on May 20, 2025

thatch commented on May 20, 2025

serhiy-storchaka commented on May 18, 2026

MorganRamsay commented on May 19, 2026

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions