Guard against negative offset/length values in tarfile's GNU sparse extraction

Bug report

Bug description:

for i in range(21):
    try:
        offset = nti(buf[pos:pos + 12])
        numbytes = nti(buf[pos + 12:pos + 24])
    except ValueError:
        break
    if offset and numbytes:
        structs.append((offset, numbytes))
    pos += 24

cpython/Lib/tarfile.py

Line 1441 in 7040aa5

if offset and numbytes:

There is no check that offset or numbytes are non-negative.
The check if offset and numbytes: only skips zero, not negative numbers.

validation should be added:

if offset >= 0 and numbytes >= 0:
    structs.append((offset, numbytes))

This will prevent the addition of invalid sparse mappings, mitigating the risk.

CPython versions tested on:

3.13

Operating systems tested on:

Windows

Linked PRs

@gpshead

Actually, I forgot about it but I think there is a similar check we now do: 7040aa5. I didn't check if this validates the structs, though so maybe extra validation is needed here (more generally, I'm not sure whether we should really stop if we encounter a ValueError rather than raising indicating a bad GNU sparse header).

cc @gpshead (maybe we should recategorize it as a security issue)

@picnixz

Thank you for analysis @picnixz

I agree that while the check in 7040aa5 is related, the explicit validation for negative values still seems necessary to be completely safe.

I agree, raising an exception to indicate a bad header would be much more robust than the current behavior.

Thank you for suggesting it be recategorized as a security issue. That was my primary concern, and I appreciate you escalating it.

@corona10

@corona10 Could you assign me on this issue?

pythongh-137396: Raise InvalidHeaderError when offset or numbytes is …

Is there ever any valid reason for any .tar file integer field to be negative?

@gpshead

@gpshead When I raise an error if n < 0 in nti() I see a single test failure in the test_read_number_fields, but looking at the issue where the possibility of negative integers was added there's an associated tar file. I downloaded and tried extracting this tar file and even this tar file extracts when disallowing negative numbers to result from nti() as the archive is simply using base-256 number encoding format but not to encode a negative integer.

I did a bunch of digging around tar standards and didn't see any mentions about disallowing negative numbers wholesale, but I think it definitely makes sense to disallow negative numbers in any framing values (size, offset).

The GNU specs say:

The sp is an array of struct sparse. Each struct sparse contains two 12-character strings which represent an offset into the file and a number of bytes to be written at that offset. The offset is absolute, and not relative to the offset in preceding array element.

AFAIK, the structs are used only in makefile:

            if tarinfo.sparse is not None:
                for offset, size in tarinfo.sparse:
                    target.seek(offset)
                    copyfileobj(source, target, size, ReadError, bufsize)
                target.seek(tarinfo.size)
                target.truncate()

Negative seek is forbidden, (it should raise). Even if copyfileobj() doesn't check size < 0, we have:

    blocks, remainder = divmod(length, bufsize)
    for b in range(blocks):
        buf = src.read(bufsize)
        if len(buf) < bufsize:
            raise exception("unexpected end of data")
        dst.write(buf)
    if remainder != 0:
        buf = src.read(remainder)
        if len(buf) < remainder:
            raise exception("unexpected end of data")
        dst.write(buf)

If length is negative, blocks is also negative, so we would skip the first loop.

where the possibility of negative integers was added there's an associated tar file

The specs allow for negative numbers but those may not necessarily be used in the sparse header data. I actually added some print() in the nti() function where negative values are handled, but the path doesn't seem to be hit (there is some hit when the converted value is 0, so this could be the bad case).

We could categorize it for now as a non-security issue (sorry for having tagged it, but as I was travelling I wasn't able to check this: I assumed that the offset could be relative and possibly have cycles when parsing but that doesn't appear to be the case).

If however, a corrupted extraction is considered a security issue (well it could, but apart from skipping some stuff I don't know how it could be exploited), we could keep the security label.

Is there ever any valid reason for any .tar file integer field to be negative?

The specs allow it, though I don't know in which case we would have it:

For fields containing numbers or timestamps that are out of range for the basic
format, the GNU format uses a base-256 representation [...] If the leading byte is
0xff (255), all the bytes of the field (including the leading byte) are concatenated
in big-endian order, with the result being a negative number expressed in two’s
complement form

@picnixz

@picnixz If there's no security risk for negative values in this case we can handle this as a non-security issue and the tag can be removed.

Yes, it can. At least I don't see a way to exploit it. Sorry for the false alarm, I wrongly assumed that it could be the same issue as the other one that was tagged as a security issue.

it was worth a look, thanks for the investigations!

Now, about how to fix this, I suggest the following:

Add a parameter to number dumping/loading functions to allow or reject negative integers. As per tar specs, some values are allowed to be negative, while others are not.
Fortunately, seek() forbids negative offsets so Python should raise it it encounters a negative offset (IOW, it's possible to parse a negative offset but it won't be usable as is). Similarly, divmod preserves the sign of the dividend so if the size is negative, we have a positive remainder with either a partial content or an incomplete read.
Raise correct errors. I suggest raising InvalidHeader (I don't remember the exact name of the exception) instead of ValueError when encountering negative framing sizes.

I think we should still backport the change to 3.9 because not all values could be passed to the _block function which has been modified in #137027. Instead, we should carefully decide which values can be negative and which cannot and raise appropriately (though _block() is only called with the .size value).

pythongh-137396: Guard against negative offset in tarfile (GH-XXXXX)

Navigation Menu

Repository navigation

Guard against negative offset/length values in tarfile's GNU sparse extraction #137396

Bug report

Bug description:

validation should be added:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Guard against negative offset/length values in tarfile's GNU sparse extraction #137396

Description

Bug report

Bug description:

validation should be added:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

picnixz commented on Aug 7, 2025

VbhvGupta commented on Aug 7, 2025

moreal commented on Aug 15, 2025

gpshead commented on Aug 17, 2025

sethmlarson commented on Aug 18, 2025

picnixz commented on Aug 18, 2025

picnixz commented on Aug 18, 2025

sethmlarson commented on Aug 25, 2025

picnixz commented on Aug 25, 2025

gpshead commented on Aug 25, 2025

picnixz commented on Aug 28, 2025

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions