Skip to content
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

Writing a small file into a ZipFile while forcing zip64 extensions results in an incorrect zip file #103861

Closed
pR0Ps opened this issue Apr 25, 2023 · 0 comments
Labels
3.11 only security fixes type-bug An unexpected behavior, bug, or error

Comments

@pR0Ps
Copy link
Contributor

pR0Ps commented Apr 25, 2023

Bug report

When forcing a small file to be added to a zipfile.Zipfile object with zip64 extensions when it doesn't need it, a Zip64 extra record is added to the resulting file, but the minimum version and filesize information are not updated to reflect this.

An example of this issue in the wild: pR0Ps/zipstream-ng#13

To reproduce:

Create the file:

import zipfile

with zipfile.ZipFile("out.zip", mode="w", allowZip64=True) as zf:
    with zf.open("text.txt", mode="w", force_zip64=True) as zi:
        zi.write(b"some data")

Using zipdetails to parse information from the resulting file (red lines are incorrect):

 0000 LOCAL HEADER #1       04034B50
-0004 Extract Zip Spec      14 '2.0'
 0005 Extract OS            00 'MS-DOS'
 0006 General Purpose Flag  0000
 0008 Compression Method    0000 'Stored'
 000A Last Mod Time         00210000 'Mon Dec 31 19:00:00 1979'
 000E CRC                   D9C2E91E
-0012 Compressed Length     00000009
-0016 Uncompressed Length   00000009
 001A Filename Length       0008
 001C Extra Length          0014
 001E Filename              'text.txt'
 0026 Extra ID #0001        0001 'ZIP64'
 0028   Length              0010
 002A   Uncompressed Size   0000000000000009
 0032   Compressed Size     0000000000000009
 003A PAYLOAD               some data
 
 0043 CENTRAL HEADER #1     02014B50
-0047 Created Zip Spec      14 '2.0'
 0048 Created OS            03 'Unix'
-0049 Extract Zip Spec      14 '2.0'
 004A Extract OS            00 'MS-DOS'
 004B General Purpose Flag  0000
 004D Compression Method    0000 'Stored'
 004F Last Mod Time         00210000 'Mon Dec 31 19:00:00 1979'
 0053 CRC                   D9C2E91E
 0057 Compressed Length     00000009
 005B Uncompressed Length   00000009
 005F Filename Length       0008
 0061 Extra Length          0000
 0063 Comment Length        0000
 0065 Disk Start            0000
 0067 Int File Attributes   0000
      [Bit 0]               0 'Binary Data'
 0069 Ext File Attributes   01800000
 006D Local Header Offset   00000000
 0071 Filename              'text.txt'
 
 0079 END CENTRAL HEADER    06054B50
 007D Number of this disk   0000
 007F Central Dir Disk no   0000
 0081 Entries in this disk  0001
 0083 Total Entries         0001
 0085 Size of Central Dir   00000036
 0089 Offset to Central Dir 00000043
 008D Comment Length        0000

In this case, the Extract Zip Spec should be 0x2D (zipfile.ZIP64_VERSION) and the Compressed/Uncompressed Lengths should both be 0xFFFFFFFF in order to defer to the lengths in the Zip64 record.

Your environment

  • CPython versions tested on: 3.12.0
  • Operating system and architecture: macOS Mojave (x86)

Linked PRs

@pR0Ps pR0Ps added the type-bug An unexpected behavior, bug, or error label Apr 25, 2023
pR0Ps added a commit to pR0Ps/cpython that referenced this issue Apr 26, 2023
This commit fixes an issue where adding a small file to a `ZipFile`
object while forcing zip64 extensions causes an extra Zip64 record to be
added to the zip, but doesn't update the `min_version` or file sizes.

Fixes python#103861
pR0Ps added a commit to pR0Ps/cpython that referenced this issue Apr 26, 2023
This commit fixes an issue where adding a small file to a `ZipFile`
object while forcing zip64 extensions causes an extra Zip64 record to be
added to the zip, but doesn't update the `min_version` or file sizes.

Fixes python#103861
pR0Ps added a commit to pR0Ps/cpython that referenced this issue Apr 27, 2023
This commit fixes an issue where adding a small file to a `ZipFile`
object while forcing zip64 extensions causes an extra Zip64 record to be
added to the zip, but doesn't update the `min_version` or file sizes.

Fixes python#103861
pR0Ps added a commit to pR0Ps/cpython that referenced this issue Apr 27, 2023
This commit fixes an issue where adding a small file to a `ZipFile`
object while forcing zip64 extensions causes an extra Zip64 record to be
added to the zip, but doesn't update the `min_version` or file sizes.

Fixes python#103861
pR0Ps added a commit to pR0Ps/cpython that referenced this issue Apr 27, 2023
This commit fixes an issue where adding a small file to a `ZipFile`
object while forcing zip64 extensions causes an extra Zip64 record to be
added to the zip, but doesn't update the `min_version` or file sizes.

Fixes python#103861
gpshead pushed a commit to gpshead/cpython that referenced this issue May 16, 2023
…ed in some cases (pythonGH-103863)

Fix Zip64 extensions not being properly applied in some cases:

Fixes an issue where adding a small file to a `ZipFile`
object while forcing zip64 extensions causes an extra Zip64 record to be
added to the zip, but doesn't update the `min_version` or file sizes in
the primary central directory header.

Also fixed an edge case in checking if zip64 extensions are required:

This fixes an issue where if data requiring zip64 extensions was added
to an unseekable stream without specifying `force_zip64=True`, zip64
extensions would not be used and a RuntimeError would not be raised when
closing the file (even though the size would be known at that point).
This would result in successfully writing corrupt zip files.

Deciding if zip64 extensions are required outside of the `FileHeader`
function means that both `FileHeader` and `_ZipWriteFile` will always be
in sync. Previously, the `FileHeader` function could enable zip64
extensions without propagating that decision to the `_ZipWriteFile`
class, which would then not correctly write the data descriptor record
or check for errors on close.

If anyone is actually using `ZipInfo.FileHeader` as a public API without
explicitly passing True or False in for zip64, their own code may still be
susceptible to that kind of bug unless they make a similar change to
where the zip64 decision happens.

Fixes pythonGH-103861

---------

Co-authored-by: Gregory P. Smith <[email protected]>.
(cherry picked from commit 798bcaa)

Co-authored-by: Carey Metcalfe <[email protected]>
@gpshead gpshead added the 3.11 only security fixes label May 16, 2023
carljm added a commit to carljm/cpython that referenced this issue May 16, 2023
* main:
  pythonGH-104510: Fix refleaks in `_io` base types (python#104516)
  pythongh-104539: Fix indentation error in logging.config.rst (python#104545)
  pythongh-104050: Don't star-import 'types' in Argument Clinic (python#104543)
  pythongh-104050: Add basic typing to CConverter in clinic.py (python#104538)
  pythongh-64595: Fix write file logic in Argument Clinic (python#104507)
  pythongh-104523: Inline minimal PGO rules (python#104524)
  pythongh-103861: Fix Zip64 extensions not being properly applied in some cases (python#103863)
  pythongh-69152: add method get_proxy_response_headers to HTTPConnection class (python#104248)
  pythongh-103763: Implement PEP 695 (python#103764)
  pythongh-104461: Run tkinter test_configure_screen on X11 only (pythonGH-104462)
  pythongh-104469: Convert _testcapi/watchers.c to use Argument Clinic (python#104503)
  pythongh-104482: Fix error handling bugs in ast.c (python#104483)
  pythongh-104341: Adjust tstate_must_exit() to Respect Interpreter Finalization (pythongh-104437)
  pythonGH-102613: Fix recursion error from `pathlib.Path.glob()` (pythonGH-104373)
gpshead added a commit that referenced this issue May 17, 2023
…some cases (GH-103863) (#104534)

Fix Zip64 extensions not being properly applied in some cases:

Fixes an issue where adding a small file to a `ZipFile`
object while forcing zip64 extensions causes an extra Zip64 record to be
added to the zip, but doesn't update the `min_version` or file sizes in
the primary central directory header.

Also fixed an edge case in checking if zip64 extensions are required:

This fixes an issue where if data requiring zip64 extensions was added
to an unseekable stream without specifying `force_zip64=True`, zip64
extensions would not be used and a RuntimeError would not be raised when
closing the file (even though the size would be known at that point).
This would result in successfully writing corrupt zip files.

Deciding if zip64 extensions are required outside of the `FileHeader`
function means that both `FileHeader` and `_ZipWriteFile` will always be
in sync. Previously, the `FileHeader` function could enable zip64
extensions without propagating that decision to the `_ZipWriteFile`
class, which would then not correctly write the data descriptor record
or check for errors on close.

If anyone is actually using `ZipInfo.FileHeader` as a public API without
explicitly passing True or False in for zip64, their own code may still be
susceptible to that kind of bug unless they make a similar change to
where the zip64 decision happens.

Fixes GH-103861

---------

.
(cherry picked from commit 798bcaa)

Co-authored-by: Carey Metcalfe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes type-bug An unexpected behavior, bug, or error
Projects
Status: Done
Development

No branches or pull requests

2 participants