GHSA-XG43-5579-QW6V: Root-cause Fix for ISDOC Decompression Bomb Handling

Vulnerability

GHSA-XG43-5579-QW6V describes uncontrolled resource consumption during extraction of untrusted ISDOCX ZIP entries and attachments, enabling denial of service via decompression bombs; the advisory summary also mentions directory traversal. The pre-patch code paths shown in commit 935fb2aa41ceddfcf43174a61a36ec620611a105 and commit 02a10123a3d5fd92950b8e4952959317c0a18952 read ZIP entries with ZipArchive::getFromName() and streamed supplement data without enforcing an attacker-controlled uncompressed-size bound first. That permits a small archive to declare very large inflated content and force memory or disk exhaustion.

The vulnerable pattern is explicit in the X reader and supplement handling: archive entries were inflated directly, and streamed writes copied until EOF without a running byte budget. PDF embedded files were also materialized without a default caller-visible cap on content retrieval. Separately, the README example previously wrote attachments using the document-declared filename directly, which is unsafe if consumers concatenate it into filesystem paths.

$xml = $zip->getFromName($files[0]);

public function saveTo(string $filename): void
{
    $resource = $this->getStream();
    $f = @fopen($filename, 'w');
    while (!feof($resource)) {
        $chunk = @fread($resource, 1 << 14);
        if ($chunk === false || @fwrite($f, $chunk) === false) {
            ...
        }
    }
}

Source-grounded impact: memory exhaustion occurs when getFromName() or PDF content decoding materializes oversized data; disk exhaustion occurs when saveTo() streams attacker-inflated content to a destination path without enforcing a maximum written size.

Patch

The patch introduces a consistent size-limiting model across ZIP-backed supplements, local supplements, and PDF embedded files. In 935fb2aa41ceddfcf43174a61a36ec620611a105, a new helper src/X/Zip.php reads central-directory entry sizes via ZipArchive::statName() so the code can reject oversized entries before inflation. src/X/Reader.php now funnels XML reads through readEntry() and enforces a 256 KiB document cap for manifest.xml and the ISDOC XML. src/X/Supplement.php adds a 32 MiB default supplement cap, checks the declared uncompressed size before inflating, and also enforces a running byte budget during saveTo() as defense in depth if metadata under-reports size.

The second patch set in 02a10123a3d5fd92950b8e4952959317c0a18952 broadens the API surface so callers can tune limits through getContents(?int $sizeLimit), getStream(?int $sizeLimit), and saveTo(string $filename, ?int $sizeLimit). It also adds PDF reader-side declared-length checks before creating supplements, exposes the PDF reader for configuration, and updates local/PDF supplement implementations to enforce the same default 32 MiB cap. New exception constructors provide explicit failure modes for oversized ZIP entries and PDF embedded files. Tests in 935fb2aa41ceddfcf43174a61a36ec620611a105 validate rejection before inflation and verify that failed writes do not leave partial files behind.

private function readEntry(ZipArchive $zip, string $name): ?string
{
    $size = Zip::entrySize($zip, $name);
    if ($size === null) {
        return null;
    }

    if ($size > self::DOCUMENT_SIZE_LIMIT) {
        throw ISDOC\ReaderException::zipEntryTooLarge($name, $size, self::DOCUMENT_SIZE_LIMIT);
    }

    $xml = $zip->getFromName($name);
    return $xml === false ? null : $xml;
}

For the traversal aspect, the README now changes the example from direct interpolation to basename($supplement->filename), which is a useful consumer-side mitigation, but this is documentation guidance rather than a library-enforced path normalization or safe-write primitive.

Review

Pros

• The ZIP decompression-bomb root cause is addressed directly by checking uncompressed entry size from the central directory before calling getFromName() or getStream(), as implemented in src/X/Zip.php and src/X/Reader.php in 935fb2aa41ceddfcf43174a61a36ec620611a105.
• src/X/Supplement.php adds defense in depth by enforcing both a pre-inflation metadata check and a running write budget during streaming. This is important because it protects disk even if ZIP metadata is malformed or under-reported.
• The patch standardizes a default 32 MiB cap across supplement accessors and save paths, reducing accidental unsafe usage by callers while still allowing explicit overrides.
• PDF handling is improved in 02a10123a3d5fd92950b8e4952959317c0a18952 by rejecting oversized embedded files from declared length before supplement construction and by re-checking decoded content length in src/PDF/Supplement.php.
• Error reporting is clearer through dedicated exceptions such as zipEntryTooLarge(), pdfSupplementTooLarge(), and supplementTooLarge(), which should improve diagnosability and safe caller handling.
• The security tests are well aligned with the advisory mechanics: they verify tiny-on-disk fixtures with oversized declared uncompressed sizes and assert rejection before inflation, which is the correct property to test for decompression bombs.
• The patch also improves adjacent security documentation around digest semantics and unsafe filename handling, reducing the chance that consumers misinterpret integrity checks as authenticity guarantees.

Cons

• The advisory summary includes directory traversal, but the patch does not appear to enforce path sanitization or destination confinement in library code. The only visible traversal mitigation in the provided sources is the README example change to basename($supplement->filename) in 02a10123a3d5fd92950b8e4952959317c0a18952. That helps consumers who copy the example, but it does not prevent unsafe direct use of document-declared filenames elsewhere.
• Some PDF protections rely on declared Length metadata before decoding, which is good as a first gate but not sufficient alone. The patch compensates with a post-decode length check in PDF\Supplement::getContents(), but the comments acknowledge that the underlying PDF library may still own whole-file memory.
• The local-file supplement implementation uses filesize() plus a post-read strlen() check. That is reasonable for local files, but it is not as strong as the ZIP streaming path because the full content may still be read into memory before the second check fires.
• The API allows null to disable limits entirely. This is intentional for flexibility, but it preserves a footgun for callers that relax limits without compensating controls.

Verdict

Partial fix.

For the decompression-bomb and uncontrolled resource-consumption portion of GHSA-XG43-5579-QW6V, the patch is technically strong and largely root-cause oriented: it moves checks ahead of inflation, adds streaming write budgets, and validates behavior with targeted tests. However, the advisory summary also mentions directory traversal, and the provided patch material does not show a library-level enforcement mechanism for safe attachment filenames or destination path confinement. Instead, it documents safer usage in README. Therefore the overall fix is best classified as partial across the full advisory scope, even though the decompression-bomb mitigation itself is well designed and credible.

References: commit 935fb2aa41ceddfcf43174a61a36ec620611a105, commit 02a10123a3d5fd92950b8e4952959317c0a18952, GitHub Security Advisory, CVE Reports summary.