From cd762a81167d6d108702ff47862bdb0dcda58b2c Mon Sep 17 00:00:00 2001 From: Alex Franchuk Date: Wed, 8 May 2024 13:12:24 +0000 Subject: [PATCH] Bug 1895599 - Fix omnijar reading with the zip crate. r=gsvelto,supply-chain-reviewers This crate is only used by the crash reporter and geckodriver, and geckodriver is only used in testing (if I understand correctly). In the future we will upstream changes to the zip crate which more gracefully handles the case which they are trying to cover. Differential Revision: https://phabricator.services.mozilla.com/D209752 --- Cargo.lock | 2 -- Cargo.toml | 4 ++++ supply-chain/config.toml | 4 ++++ third_party/rust/zip/src/read.rs | 9 +++------ toolkit/crashreporter/client/app/src/lang/omnijar.rs | 3 ++- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a1e434c4bd11..c7a97981f4dd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7043,8 +7043,6 @@ dependencies = [ [[package]] name = "zip" version = "0.6.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0445d0fbc924bb93539b4316c11afb121ea39296f99a3c4c9edad09e3658cdef" dependencies = [ "byteorder", "crc32fast", diff --git a/Cargo.toml b/Cargo.toml index ca9b735fb431..b55ab4e8e114 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -188,6 +188,10 @@ web-sys = { path = "build/rust/dummy-web/web-sys" } # Overrides to allow easier use of common internal crates. moz_asserts = { path = "mozglue/static/rust/moz_asserts" } + +# Patched version of zip 0.6.4 to allow for reading omnijars. +zip = { path = "third_party/rust/zip" } + # Patch `rure` to disable building the cdylib and staticlib targets # Cargo has no way to disable building targets your dependencies provide which # you don't depend on, and linking the cdylib breaks during instrumentation diff --git a/supply-chain/config.toml b/supply-chain/config.toml index dd3695f300ab..5ace40a011b7 100644 --- a/supply-chain/config.toml +++ b/supply-chain/config.toml @@ -243,6 +243,10 @@ notes = "Local override of the crates.io crate that uses a non-vendored local co [policy.wr_malloc_size_of] audit-as-crates-io = false +[policy.zip] +audit-as-crates-io = true +notes = "Locally patched version of the zip crate to allow for reading omnijars." + [[exemptions.ahash]] version = "0.7.6" criteria = "safe-to-deploy" diff --git a/third_party/rust/zip/src/read.rs b/third_party/rust/zip/src/read.rs index dad20c260b27..3f3f41010cd3 100644 --- a/third_party/rust/zip/src/read.rs +++ b/third_party/rust/zip/src/read.rs @@ -334,13 +334,10 @@ impl ZipArchive { // offsets all being too small. Get the amount of error by comparing // the actual file position we found the CDE at with the offset // recorded in the CDE. - let archive_offset = cde_start_pos - .checked_sub(footer.central_directory_size as u64) - .and_then(|x| x.checked_sub(footer.central_directory_offset as u64)) - .ok_or(ZipError::InvalidArchive( - "Invalid central directory size or offset", - ))?; + // Bug 1895599: omnijars nor other zips we read have data prepended to them; trust + // the offsets! + let archive_offset = 0; let directory_start = footer.central_directory_offset as u64 + archive_offset; let number_of_files = footer.number_of_files_on_this_disk as usize; Ok((archive_offset, directory_start, number_of_files)) diff --git a/toolkit/crashreporter/client/app/src/lang/omnijar.rs b/toolkit/crashreporter/client/app/src/lang/omnijar.rs index 2e26dc29f624..f3a9ab47422a 100644 --- a/toolkit/crashreporter/client/app/src/lang/omnijar.rs +++ b/toolkit/crashreporter/client/app/src/lang/omnijar.rs @@ -31,9 +31,10 @@ pub fn read() -> anyhow::Result { .ok_or(anyhow::anyhow!("multilocale file was empty"))? .context("failed to read first line of multilocale file")? }; + let mut file = zip .by_name(&format!( - "localization/{locale}/toolkit/crashreporter/crashreporter.ftl" + "localization/{locale}/crashreporter/crashreporter.ftl" )) .with_context(|| format!("failed to locate localization file for {locale}"))?;