Unbounded reads by zipfile may cause a MemoryError.

Bug report

Bug description:

def _EndRecData(fpin):
    """Return data from the "End of Central Directory" record, or None.

    The data is a list of the nine items in the ZIP "End of central dir"
    record followed by a tenth item, the file seek offset of this record."""

    # Determine file size
    fpin.seek(0, 2)
    filesize = fpin.tell()

    # Check to see if this is ZIP file with no archive comment (the
    # "end of central directory" structure should be the last item in the
    # file if this is the case).
    try:
        fpin.seek(-sizeEndCentDir, 2)
    except OSError:
        return None
    data = fpin.read()
    if (len(data) == sizeEndCentDir and
        data[0:4] == stringEndArchive and
        data[-2:] == b"\000\000"):

When checking whether a file is a zip file, MemoryError was triggered, followed by OOM. After investigation, it was found that it was a read() read exception.

Through PDB debugging, it was found that a link file was read, which points to /proc/kcore, why does the existing zip file check not determine whether it is a zip file by reading the header byte (504B0304) of the file .

I think the existing judgment ZIP method does not limit the read reading. When reading a non -normal file, it may cause the system to collapse .

Hope to be resolved.

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

That code already tries to check if the file is a zipfile by reading the header at the end of the file. The MemoryError seems to indicate that seeking to the end of the file doesn't work as expected for /proc/kcore.

This function could be written a bit more defensively though, for example by using fpin.read(sizeEndCentDir+1) instead of fpin.read().

This implements the suggested change: #122101

There's a secondary thing here, that unbounded read defaults to the size at open with #120755. In this bug that's more than the amount of RAM remaining on the machine, hence the MemoryError / OOM. That stashed size is currently invalidated on .truncate() but not on .seek() which zipfile uses. I'd like to make seek() clear the estimated size, but that change conflicts a lot with #121593, so have been holding off.

#122101 I think resolves this case for zipfile (and makes it more predictable, safer behavior generally) but I suspect the .seek() case will come up more in library code, so before python 3.14 would like to get that to invalidate the cached size generally.

@gpshead

Are (potential) DDoS like that considered as security issues or not? @gpshead

gh-113977, gh-120754: Remove unbounded reads from zipfile (GH-122101)

pythongh-113977, pythongh-120754: Remove unbounded reads from zipfile (…

@gpshead

Are (potential) DDoS like that considered as security issues or not? @gpshead

They can be, but not particularly severe. I don't think this one is worth considering a security problem because the circumstances in which it can occur are extremely limited: Someone has to have opened a file that either isn't seekable or provides an egregious amount of data upon read after seeking to the end.

Just constructing that scenario in the first place is a sign of greater problems in the system.

@sethmlarson as FYI

Thanks for the nice bug report and PRs. The 3.12 and 3.13 back ports are also set to auto merge. Indeed, avoiding unbounded read assumptions is the right coding practice.

[3.12] gh-113977, gh-120754: Remove unbounded reads from zipfile (GH-…

[3.13] gh-113977, gh-120754: Remove unbounded reads from zipfile (GH-…

pythongh-113977, pythongh-120754: Remove unbounded reads from zipfile (…

Navigation Menu

Repository navigation

Unbounded reads by `zipfile` may cause a `MemoryError`. #113977

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Unbounded reads by zipfile may cause a MemoryError. #113977

Description

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

ronaldoussoren commented on Jan 12, 2024

danifus commented on Aug 5, 2024

cmaloney commented on Aug 7, 2024

picnixz commented on Nov 2, 2024

gpshead commented on Nov 3, 2024

gpshead commented on Nov 3, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

Unbounded reads by `zipfile` may cause a `MemoryError`. #113977