Skip to content

v0.10.0

Choose a tag to compare

@magifd2 magifd2 released this 02 Aug 03:46

A security review of the extractor, and the five defects it found. Two of
them were crashes on malformed input; two were holes in guarantees the app
already claimed (Gatekeeper propagation, "never overwrite"); one let an
archive decide the permissions of the files it dropped on you.

Changed

  • Hardening pass over the unchecked arithmetic and boundaries the two
    crashes came from
    (#6): the little-endian readers are bounds-checked
    and throwing, so a read added to the parser later cannot forget the
    guard; entry ranges, the writer's 16/32-bit header conversions and the
    extraction budget all reject values that would trap; archives and
    extracted files are created with O_EXCL, so "never overwrite" no
    longer rests on the gap between the check and the write; AES key
    material is wiped once it stops being needed

Fixed

  • A malformed ZIP64 header crashed the process instead of being
    rejected
    (#1). Two bounds checks in the central-directory parser did
    their own arithmetic on attacker-supplied 64-bit values: the ZIP64
    locator address (eocd - 20, underflowing on a 22-byte file whose EOCD
    claims ZIP64) and the directory bounds (cdOffset + cdSize, overflowing
    while checking themselves). Both trapped, taking the GUI down on a
    double-clicked .zip. The checks now subtract from the file size instead
    of adding to the offsets, and two hostile fixtures cover the shapes

  • The pre-extraction free-space budget could be switched off from inside
    the archive
    (#2). The declared total was summed with wrapping
    arithmetic, so two ZIP64 entries declaring 2^63 apiece summed to exactly
    zero and the budget check approved anything. Since per-entry fail-fast
    only bounds an entry by its own declared size, that left a decompression
    bomb free to fill the volume. The sum now saturates at UInt64.max, and
    the check subtracts the margin from the free space instead of adding it
    to the requirement

  • Quarantine did not reach directories the extractor created
    implicitly
    (#3), so an archive with no directory entries — 7-Zip writes
    them that way routinely — produced a .app whose files each carried
    com.apple.quarantine but whose bundle root, the thing Gatekeeper
    actually evaluates, did not. ditto marks the bundle root; now so do we.
    This was the ADR-012 §4 gap re-opening through a different door

  • Extraction applied the archive's permission bits verbatim (#4), so an
    archive asking for 0777 produced world-writable — and executable —
    files in the user's folder. On a shared Mac another local account could
    rewrite them. The requested mode is now masked with the process umask,
    the rule unzip and ditto follow (07770755, 06660644
    under the default umask); setuid/setgid never survived and still do not

  • A failure to mark an extracted item as quarantined was silent (#5),
    and the result claimed propagation had happened as long as the archive
    carried the attribute. Failures are now collected per item and reported
    the way every other security-relevant outcome is: a warning line in the
    CLI, and the result dialog (never the quiet notification) in the GUI