[yuxi-liu-wired]

• The PR title must conform to Conventional Commits and start with one of the types specified by the Angular convention.

Problem

ZipStreamReader::visit never calls ZipStreamVisitor::visit_additional_metadata, so
ZipStreamReader::extract never applies the unix permissions from the central directory, though
its documentation says the permissions are applied after the files are created. Every file and
directory gets the process default instead: a 0o600 file comes out 0o644 (umask 0022),
readable by every local user, and a 0o700 directory comes out listable.
ZipArchive::extract on the same archive restores the modes.

The cause:

1. read_zipfile_from_stream reads 4 bytes, sees the central directory header signature and
   returns Ok(None). The stream is now 4 bytes into the first central directory header.
2. visit then calls parse_central_directory, which reads the signature again: it gets the
   next 4 bytes of the header body and fails with "Invalid Central Directory header".
3. while let Ok(metadata) = self.parse_central_directory() swallows the error, so visit
   returns Ok(()) without calling visit_additional_metadata.

The in-crate test zip_read_streaming_visitor only asserts inside
visit_additional_metadata, so it passes when that method is never called. Every release with
read::stream is affected (checked on 0.6.5, 8.6.0 and master).

Why the extraction side changes too

Fixing only the offset would run Extractor::visit_additional_metadata for the first time, and as
written it is not safe: it applies the mode to destination.join(central directory name) with
fs::set_permissions, which follows symlinks. zip2 does not check central directory names against
the local headers, so a central directory entry can name a path that no local entry wrote. With a
symlink already in the destination (dest/a -> ../outside), an entry whose local name is
c_d.txt and whose central name is a/b.txt with mode 0o777 would chmod outside/b.txt,
while visit_file correctly refuses to write through that link.

Change

• visit puts the consumed signature back in front of the stream before parsing the central
  directory, so every header parses.
• extract records, for each local entry, the path visit_file checked (safe_prepare_path)
  and wrote. A central directory mode applies only to that path; a name this pass did not write
  gets no mode.
• Modes are applied after the visit, children first (as ZipArchive::extract does), and never
  through a symlink (symlink_metadata check before set_permissions).

Like ZipArchive::extract, this does not try to protect against another process changing the
destination during the extraction.

Tests

In tests/stream.rs:

• visit_sees_every_central_directory_entry: 3 local and 3 central entries (master: 0 central).
• extract_applies_unix_modes: 751 / 600 / 644 (master: the process default).
• extract_ignores_modes_for_names_it_did_not_write: the reproduction above; fails if a name
  only the central directory carries gets a mode.
• extract_ignores_modes_for_a_name_that_is_a_symlink_it_did_not_write: the central-only name
  is itself a pre-existing symlink to a file outside; fails if, in addition, the symlink check is
  removed.

The suite passes on 1.88, stable and nightly with default features, --no-default-features,
--all-features, and the two single-deflate feature sets; cargo clippy --all-targets -- -D warnings (default, --no-default-features, --all-features) and cargo doc --no-deps are
clean; cargo fmt --check --all is clean.

Found with a fuzzer that drives both extraction paths: read/stream.rs stayed at 63% line
coverage because the central directory pass never ran.

[Pr0methean]

Good start but still needs some work.

[Pr0methean]

@yuxi-liu-wired This is failing tests and will probably do so even once #1035 is fixed and merged.

[yuxi-liu-wired]

@Pr0methean I checked this locally. Both red checks on b90d6c3 come from master, not from this PR (which only touches src/read/stream.rs and tests/stream.rs):

• check_minimal_versions: test_cannot_create_symlink_pointing_outside_destination and test_cannot_symlink_outside_destination (tests/extract_symlink.rs) panic at src/path.rs:243, the same as on master 96c1693.
• Test endianness platform (miri): tests/extract_symlink_options.rs calls mkdir, which miri rejects under isolation.

With #1035 (7b2fa0b) merged into this branch, the one-line conflict in stream.rs resolved as &self.resolved_paths, SymlinkExtractAction::ExtractInFolder, cargo test --all-features passes everything except test_extract_options_recursive_in_folder (tests/extract_symlink_options.rs:253, InvalidArchive("Path escapes the destination directory")). That test fails the same way on #1035 alone, so it isn't from this PR either. Happy to rebase onto master once #1035 lands.

[yuxi-liu-wired]

The endianness job failed after the master merge: since #1035 it runs the tests under miri, and this PR's three file-system tests in tests/stream.rs create a temp dir, which miri's isolation doesn't allow. I marked them #[cfg(all(unix, not(miri)))] like #1035 did for the symlink tests. Under cargo +nightly miri test --target aarch64_be-unknown-linux-gnu --no-default-features --test stream the other five pass, and natively all eight still run.

[Pr0methean]

@yuxi-liu-wired Could you please share the fuzzers you used to find this bug and the ones in #1024, #1025, #1027 and #1028? I'm wondering if we might be able to use some of their code to increase the coverage of our existing fuzzers.

[yuxi-liu-wired]

@Pr0methean sure, here they are: https://gist.github.com/yuxi-liu-wired/ce659d5876d86056ff82621b9af1d8a1

They're five cargo-fuzz targets that check zip2's documented promises as end-state invariants, not just "no panic". Each file's header lists its checks. The README maps each of these PRs to the target that found it: #1024 came from zipcrypt, and #1025, #1027 and #1028 from zipapi. #1030 is the exception: a coverage audit showed the stream reader's central-directory code never ran, and reading it found the bug. I rebuilt them against current master and ran each for a few minutes before posting: all five came back clean in 2-minute runs from an empty corpus (zipread needs seed archives to get far, see the README). Feel free to take any of the code.