Extract archives in pure Lua with a vendored zzlib #4

2026-09-10T22:48:39+10:00

unkin-agent commented

2026-09-10 22:48:39 +10:00

Installing a plugin needed unzip or tar on PATH, which rules out machines
without them, and left archive path safety entirely to those tools.

Vendor zzlib (codeberg.org/zerkman/zzlib @ 9c39321, WTFPL) under lua/arti-pack/vendor
Add arti-pack.archive: a zip reader over zzlib inflate, a tar reader over its gunzip
Reject absolute, parent-walking, backslashed and NUL-bearing entry names
Reject entries that are not regular files or directories, and encrypted entries
Verify every zip entry CRC32 and size; fail the install on mismatch
Carry only the executable bit from archive modes
Replace fs.extract_cmd with fs.extract; drop unzip from the test pipeline and docs

Installing a plugin needed unzip or tar on PATH, which rules out machines without them, and left archive path safety entirely to those tools. - Vendor zzlib (codeberg.org/zerkman/zzlib @ 9c39321, WTFPL) under lua/arti-pack/vendor - Add arti-pack.archive: a zip reader over zzlib inflate, a tar reader over its gunzip - Reject absolute, parent-walking, backslashed and NUL-bearing entry names - Reject entries that are not regular files or directories, and encrypted entries - Verify every zip entry CRC32 and size; fail the install on mismatch - Carry only the executable bit from archive modes - Replace fs.extract_cmd with fs.extract; drop unzip from the test pipeline and docs

unkin-agent added 2 commits 2026-09-10 22:48:39 +10:00

Vendor zzlib for pure-Lua DEFLATE bae3a517f7

Add zzlib (codeberg.org/zerkman/zzlib @ 9c39321, WTFPL) under
lua/arti-pack/vendor. Namespace its submodule requires and re-export
crc32; the inflate backends are unmodified.

Exclude the vendor directory from luacheck and stylua.

Extract archives in pure Lua

ci/woodpecker/pr/test Pipeline was successful

Details

ci/woodpecker/pr/lint Pipeline was successful

Details

ci/woodpecker/pr/pre-commit Pipeline was successful

Details

5809a843c8

Installing a plugin no longer needs unzip or tar on PATH, and entry paths
are vetted in Lua instead of trusting an external tool.

Add arti-pack.archive: a zip reader over zzlib's inflate and a tar reader
over its gunzip.
Reject absolute, parent-walking, backslashed and NUL-bearing entry names,
and any entry that is not a regular file or directory.
Verify every zip entry's CRC32 and size, and fail the install on mismatch.
Carry only the executable bit from archive modes.
Replace fs.extract_cmd with fs.extract and call it synchronously.
Drop unzip from the test pipeline and the stated requirements.

unkin-agent commented

2026-09-10 23:03:35 +10:00

Reviewed by building hostile zip/tar fixtures byte-by-byte (Python) and running the real archive.lua against them, plus a real-world equivalence test.

Verified clean (no repro found):

Path safety holds for ../ (shallow/deep/mixed), absolute paths, C: drive, backslashes, embedded NUL, empty/./../normalizes-to-parent names, symlink/hardlink/device/fifo/setuid entries (zip external_attr and tar typeflags), encrypted zip entries, GNU-L / pax-x / ustar-prefix tar path overrides — all rejected, nothing written outside dest in any case.
zip64 rejected via locator-signature, entry-count-0xffff, and cd-offset-sentinel fixtures (all 3 constructed manually and confirmed rejected).
CRC32 + size checked and enforced for both stored and deflated zip entries (built a real deflate stream with a flipped CRC and a lied-about usize — both rejected).
Truncated archives, lying central-directory counts, corrupt gzip/tar checksums, base-256 tar sizes — all rejected loudly.
plenary.nvim v0.1.4 zip (160 entries, 125 deflated) and tar.gz extracted with old unzip/tar vs. new code: diff -rq and file-mode listings identical.
Vendored zzlib.lua/inflate-bit32.lua/inflate-bwo.lua diffed byte-for-byte against upstream 9c39321: only the documented header comment + the two documented changes (namespaced requires, crc32 re-export) — nothing silently edited.
luacheck lua tests = 0 warnings (ran for real in a container); the MODE_EXEC/MODE_FILE/MODE_EXEC_FILE "unused locals" concern doesn't reproduce — all three are used (archive.lua:91,229,333). stylua --check passes. Full suite (119 tests) passes, and still passes with unzip/tar/zip removed from PATH entirely.
archive_root() untouched; commits properly split (vendor lib / integration); PR body within the line/char cap.

Two non-security nits (nice-to-have, not blocking):

lua/arti-pack/archive.lua:159 (zip_entries) — a central-directory entry truncated right before its namelen/extralen fields makes u16() return nil, so pos = pos + 46 + namelen + u16(buf,pos+30) + u16(buf,pos+32) raises a raw Lua error ("attempt to perform arithmetic on a nil value") instead of the module's usual nil, "malformed central directory". fs.extract's pcall wrapper catches it today so nothing crashes in the real install path, but archive.unzip/archive.extract called directly throws, and the surfaced message is an internal error string instead of a clean one. Suggest adding a pos + 46 > #buf bounds check next to the existing u32(buf,pos) ~= CD_SIG guard so this fails the same clean way as the other malformed-CD cases.
tests/archive_spec.lua — the zipf() fixture builder hardcodes method=0 (STORED) for every entry, so no test ever builds a DEFLATE (method=8) zip entry: the actual zzlib inflate path — the highest-risk code here, since it parses attacker-controlled compressed bytes — has zero direct test coverage. "fails loudly on a CRC mismatch" only exercises the stored path. The existing gz() helper already proves a stored-block (BTYPE=00) DEFLATE stream is enough to build valid compressed data without a real compressor; the same trick would let zipf() support a deflated entry and a deflated CRC/size-mismatch case.

Reviewed by building hostile zip/tar fixtures byte-by-byte (Python) and running the real `archive.lua` against them, plus a real-world equivalence test. **Verified clean (no repro found):** - Path safety holds for `../` (shallow/deep/mixed), absolute paths, `C:` drive, backslashes, embedded NUL, empty/`.`/`..`/normalizes-to-parent names, symlink/hardlink/device/fifo/setuid entries (zip external_attr and tar typeflags), encrypted zip entries, GNU-L / pax-x / ustar-prefix tar path overrides — all rejected, nothing written outside `dest` in any case. - zip64 rejected via locator-signature, entry-count-0xffff, and cd-offset-sentinel fixtures (all 3 constructed manually and confirmed rejected). - CRC32 + size checked and enforced for **both** stored and deflated zip entries (built a real deflate stream with a flipped CRC and a lied-about usize — both rejected). - Truncated archives, lying central-directory counts, corrupt gzip/tar checksums, base-256 tar sizes — all rejected loudly. - `plenary.nvim` v0.1.4 zip (160 entries, 125 deflated) and tar.gz extracted with old `unzip`/`tar` vs. new code: `diff -rq` and file-mode listings identical. - Vendored `zzlib.lua`/`inflate-bit32.lua`/`inflate-bwo.lua` diffed byte-for-byte against upstream `9c39321`: only the documented header comment + the two documented changes (namespaced requires, crc32 re-export) — nothing silently edited. - `luacheck lua tests` = 0 warnings (ran for real in a container); the `MODE_EXEC`/`MODE_FILE`/`MODE_EXEC_FILE` "unused locals" concern doesn't reproduce — all three are used (archive.lua:91,229,333). `stylua --check` passes. Full suite (119 tests) passes, and still passes with `unzip`/`tar`/`zip` removed from PATH entirely. - `archive_root()` untouched; commits properly split (vendor lib / integration); PR body within the line/char cap. **Two non-security nits (nice-to-have, not blocking):** 1. `lua/arti-pack/archive.lua:159` (`zip_entries`) — a central-directory entry truncated right before its namelen/extralen fields makes `u16()` return `nil`, so `pos = pos + 46 + namelen + u16(buf,pos+30) + u16(buf,pos+32)` raises a raw Lua error ("attempt to perform arithmetic on a nil value") instead of the module's usual `nil, "malformed central directory"`. `fs.extract`'s `pcall` wrapper catches it today so nothing crashes in the real install path, but `archive.unzip`/`archive.extract` called directly throws, and the surfaced message is an internal error string instead of a clean one. Suggest adding a `pos + 46 > #buf` bounds check next to the existing `u32(buf,pos) ~= CD_SIG` guard so this fails the same clean way as the other malformed-CD cases. 2. `tests/archive_spec.lua` — the `zipf()` fixture builder hardcodes `method=0` (STORED) for every entry, so no test ever builds a DEFLATE (method=8) zip entry: the actual zzlib inflate path — the highest-risk code here, since it parses attacker-controlled compressed bytes — has zero direct test coverage. "fails loudly on a CRC mismatch" only exercises the stored path. The existing `gz()` helper already proves a stored-block (BTYPE=00) DEFLATE stream is enough to build valid compressed data without a real compressor; the same trick would let `zipf()` support a deflated entry and a deflated CRC/size-mismatch case.

benvin merged commit 04506285df into main

2026-09-10 23:18:13 +10:00

benvin deleted branch benvin/vendor-zzlib

2026-09-10 23:18:13 +10:00

~~unkin-agent referenced this pull request 2026-09-10 23:28:47 +10:00~~

Cover the DEFLATE path and stop malformed zips raising #5

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/arti-pack#4