diff --git a/Cargo.lock b/Cargo.lock index 1a85011..8b5bef8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -28,12 +28,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "ambient-authority" -version = "0.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e9d4ee0d472d1cd2e28c97dfa124b3d8d992e10eb0a035f33f5d12e3a177ba3b" - [[package]] name = "anstream" version = "0.6.20" @@ -121,8 +115,6 @@ dependencies = [ "bitflags", "bstr", "bzip2", - "cap-std", - "cap-tempfile", "clap", "clap_complete", "cms", @@ -250,49 +242,6 @@ dependencies = [ "libbz2-rs-sys", ] -[[package]] -name = "cap-primitives" -version = "3.4.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0a1e394ed14f39f8bc26f59d4c0c010dbe7f0a1b9bafff451b1f98b67c8af62a" -dependencies = [ - "ambient-authority", - "fs-set-times", - "io-extras", - "io-lifetimes", - "ipnet", - "maybe-owned", - "rustix", - "rustix-linux-procfs", - "windows-sys 0.59.0", - "winx", -] - -[[package]] -name = "cap-std" -version = "3.4.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "07c0355ca583dd58f176c3c12489d684163861ede3c9efa6fd8bba314c984189" -dependencies = [ - "cap-primitives", - "io-extras", - "io-lifetimes", - "rustix", -] - -[[package]] -name = "cap-tempfile" -version = "3.4.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9bdc50d18ee6c3551b30eb7ad5c4628d7c73ed9e1696b63c432a55602d634d7d" -dependencies = [ - "cap-std", - "rand", - "rustix", - "rustix-linux-procfs", - "uuid", -] - [[package]] name = "cbc" version = "0.1.2" @@ -662,17 +611,6 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" -[[package]] -name = "fs-set-times" -version = "0.20.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "94e7099f6313ecacbe1256e8ff9d617b75d1bcb16a6fddef94866d225a01a14a" -dependencies = [ - "io-lifetimes", - "rustix", - "windows-sys 0.59.0", -] - [[package]] name = "fuzz" version = "3.19.0" @@ -806,28 +744,6 @@ dependencies = [ "generic-array", ] -[[package]] -name = "io-extras" -version = "0.18.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2285ddfe3054097ef4b2fe909ef8c3bcd1ea52a8f0d274416caebeef39f04a65" -dependencies = [ - "io-lifetimes", - "windows-sys 0.59.0", -] - -[[package]] -name = "io-lifetimes" -version = "2.0.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "06432fb54d3be7964ecd3649233cddf80db2832f47fec34c01f65b3d9d774983" - -[[package]] -name = "ipnet" -version = "2.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "469fb0b9cefa57e3ef31275ee7cacb78f2fdca44e4765491884a2b119d4eb130" - [[package]] name = "is_terminal_polyfill" version = "1.70.1" @@ -843,16 +759,6 @@ dependencies = [ "either", ] -[[package]] -name = "js-sys" -version = "0.3.77" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1cfaf33c695fc6e08064efbc1f72ec937429614f25eef83af942d0e227c3a28f" -dependencies = [ - "once_cell", - "wasm-bindgen", -] - [[package]] name = "lazy_static" version = "1.5.0" @@ -954,12 +860,6 @@ dependencies = [ "sha2", ] -[[package]] -name = "maybe-owned" -version = "0.3.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4facc753ae494aeb6e3c22f839b158aebd4f9270f55cd3c79906c45476c47ab4" - [[package]] name = "memchr" version = "2.7.5" @@ -1496,22 +1396,6 @@ dependencies = [ "windows-sys 0.60.2", ] -[[package]] -name = "rustix-linux-procfs" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2fc84bf7e9aa16c4f2c758f27412dc9841341e16aa682d9c7ac308fe3ee12056" -dependencies = [ - "once_cell", - "rustix", -] - -[[package]] -name = "rustversion" -version = "1.0.21" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a0d197bd2c9dc6e53b84da9556a69ba4cdfab8619eb41a8bd1cc2027a0f6b1d" - [[package]] name = "salsa20" version = "0.10.2" @@ -1889,17 +1773,6 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" -[[package]] -name = "uuid" -version = "1.17.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3cf4199d1e5d15ddd86a694e4d0dffa9c323ce759fea589f00fef9d81cc1931d" -dependencies = [ - "getrandom 0.3.3", - "js-sys", - "wasm-bindgen", -] - [[package]] name = "valuable" version = "0.1.1" @@ -1927,64 +1800,6 @@ dependencies = [ "wit-bindgen-rt", ] -[[package]] -name = "wasm-bindgen" -version = "0.2.100" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1edc8929d7499fc4e8f0be2262a241556cfc54a0bea223790e71446f2aab1ef5" -dependencies = [ - "cfg-if", - "once_cell", - "rustversion", - "wasm-bindgen-macro", -] - -[[package]] -name = "wasm-bindgen-backend" -version = "0.2.100" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f0a0651a5c2bc21487bde11ee802ccaf4c51935d0d3d42a6101f98161700bc6" -dependencies = [ - "bumpalo", - "log", - "proc-macro2", - "quote", - "syn 2.0.104", - "wasm-bindgen-shared", -] - -[[package]] -name = "wasm-bindgen-macro" -version = "0.2.100" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7fe63fc6d09ed3792bd0897b314f53de8e16568c2b3f7982f468c0bf9bd0b407" -dependencies = [ - "quote", - "wasm-bindgen-macro-support", -] - -[[package]] -name = "wasm-bindgen-macro-support" -version = "0.2.100" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ae87ea40c9f689fc23f209965b6fb8a99ad69aeeb0231408be24920604395de" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.104", - "wasm-bindgen-backend", - "wasm-bindgen-shared", -] - -[[package]] -name = "wasm-bindgen-shared" -version = "0.2.100" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a05d73b933a847d6cccdda8f838a22ff101ad9bf93e33684f39c1f5f0eece3d" -dependencies = [ - "unicode-ident", -] - [[package]] name = "winapi" version = "0.3.9" @@ -2178,16 +1993,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "winx" -version = "0.36.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3f3fd376f71958b862e7afb20cfe5a22830e1963462f3a17f49d82a6c1d1f42d" -dependencies = [ - "bitflags", - "windows-sys 0.59.0", -] - [[package]] name = "wit-bindgen-rt" version = "0.39.0" diff --git a/avbroot/Cargo.toml b/avbroot/Cargo.toml index 46d0872..c603ebb 100644 --- a/avbroot/Cargo.toml +++ b/avbroot/Cargo.toml @@ -14,8 +14,6 @@ base64 = "0.22.1" bitflags = { version = "2.4.1", features = ["serde"] } bstr = "1.6.2" bzip2 = "0.6.0" -cap-std = "3.0.0" -cap-tempfile = "3.0.0" clap = { version = "4.4.1", features = ["derive"] } clap_complete = "4.4.0" cms = { version = "0.2.2", features = ["std"] } diff --git a/avbroot/src/cli/avb.rs b/avbroot/src/cli/avb.rs index 12be852..3ccfee3 100644 --- a/avbroot/src/cli/avb.rs +++ b/avbroot/src/cli/avb.rs @@ -1,20 +1,16 @@ -// SPDX-FileCopyrightText: 2023-2024 Andrew Gunnerson +// SPDX-FileCopyrightText: 2023-2025 Andrew Gunnerson // SPDX-License-Identifier: GPL-3.0-only use std::{ collections::{HashMap, HashSet}, - ffi::{OsStr, OsString}, - fs::{self, File}, + ffi::OsString, + fs::{self, File, OpenOptions}, io::{self, BufReader, BufWriter, Cursor, Seek, SeekFrom, Write}, path::{Path, PathBuf}, sync::atomic::AtomicBool, }; use anyhow::{Context, Result, anyhow, bail}; -use cap_std::{ - ambient_authority, - fs::{Dir, OpenOptions}, -}; use clap::{Args, Parser, Subcommand}; use rayon::prelude::{IntoParallelRefIterator, ParallelIterator}; use rsa::RsaPublicKey; @@ -410,20 +406,11 @@ fn display_info(display: &DisplayGroup, info: &AvbInfo) { } } -/// Ensure that the partition name won't cause directory traversals. -fn ensure_name_is_safe(name: &str) -> Result<()> { - if Path::new(name).file_name() != Some(OsStr::new(name)) { - bail!("Unsafe partition name: {name}"); - } - - Ok(()) -} - /// Recursively verify an image's vbmeta header and all of the chained images. /// `seen` is used to prevent cycles. `descriptors` will contain all of the hash /// and hash tree descriptors that need to be verified. pub fn verify_headers( - directory: &Dir, + directory: &Path, name: &str, expected_key: Option<&RsaPublicKey>, seen: &mut HashSet, @@ -433,12 +420,9 @@ pub fn verify_headers( return Ok(()); } - ensure_name_is_safe(name)?; - - let path = format!("{name}.img"); - let raw_reader = directory - .open(&path) - .with_context(|| format!("Failed to open for reading: {path:?}"))?; + let path = util::path_join_single(directory, format!("{name}.img"))?; + let raw_reader = + File::open(&path).with_context(|| format!("Failed to open for reading: {path:?}"))?; let (header, _, _) = avb::load_image(BufReader::new(raw_reader)) .with_context(|| format!("Failed to load vbmeta structures: {path:?}"))?; @@ -545,7 +529,7 @@ fn verify_and_repair( /// Verify hash and hash tree descriptor digests and FEC data against their /// corresponding input files. pub fn verify_descriptors( - directory: &Dir, + directory: &Path, descriptors: &HashMap, repair: bool, cancel_signal: &AtomicBool, @@ -557,10 +541,12 @@ pub fn verify_descriptors( .map(|(name, descriptor)| { let _span = parent_span.enter(); - let path = format!("{name}.img"); - let file = match directory - .open_with(&path, OpenOptions::new().read(true).write(repair)) - .map(|f| PSeekFile::new(f.into_std())) + let path = util::path_join_single(directory, format!("{name}.img"))?; + let file = match OpenOptions::new() + .read(true) + .write(repair) + .open(&path) + .map(PSeekFile::new) { Ok(f) => f, // Some devices, like bluejay, have vbmeta descriptors that @@ -587,7 +573,7 @@ pub fn verify_descriptors( } fn compute_digest_recursive( - directory: &Dir, + directory: &Path, name: &str, context: &mut ring::digest::Context, max_depth: u8, @@ -602,11 +588,8 @@ fn compute_digest_recursive( seen.insert(name.to_owned()); - ensure_name_is_safe(name)?; - - let path = format!("{name}.img"); - let mut raw_reader = directory - .open(&path) + let path = util::path_join_single(directory, format!("{name}.img"))?; + let mut raw_reader = File::open(&path) .map(BufReader::new) .with_context(|| format!("Failed to open for reading: {path:?}"))?; let (header, footer, _) = avb::load_image(&mut raw_reader) @@ -658,7 +641,11 @@ fn compute_digest_recursive( /// the root vbmeta image, followed by the headers in the immediate chained /// partitions. This digest is not defined to be recursive, so headers of /// chained partitions more than one level deep are ignored. -pub fn compute_digest(directory: &Dir, name: &str, cancel_signal: &AtomicBool) -> Result<[u8; 32]> { +pub fn compute_digest( + directory: &Path, + name: &str, + cancel_signal: &AtomicBool, +) -> Result<[u8; 32]> { let mut seen = HashSet::::new(); let mut context = ring::digest::Context::new(&ring::digest::SHA256); @@ -776,10 +763,7 @@ fn verify_subcommand(cli: &VerifyCli, cancel_signal: &AtomicBool) -> Result<()> None }; - let authority = ambient_authority(); - let parent_path = util::parent_path(&cli.input); - let directory = Dir::open_ambient_dir(parent_path, authority) - .with_context(|| format!("Failed to open directory: {parent_path:?}"))?; + let directory = util::parent_path(&cli.input); let name = cli .input .file_stem() @@ -791,13 +775,13 @@ fn verify_subcommand(cli: &VerifyCli, cancel_signal: &AtomicBool) -> Result<()> let mut descriptors = HashMap::::new(); verify_headers( - &directory, + directory, name, public_key.as_ref(), &mut seen, &mut descriptors, )?; - verify_descriptors(&directory, &descriptors, cli.repair, cancel_signal)?; + verify_descriptors(directory, &descriptors, cli.repair, cancel_signal)?; info!("Successfully verified all vbmeta signatures and hashes"); @@ -805,10 +789,7 @@ fn verify_subcommand(cli: &VerifyCli, cancel_signal: &AtomicBool) -> Result<()> } fn digest_subcommand(cli: &DigestCli, cancel_signal: &AtomicBool) -> Result<()> { - let authority = ambient_authority(); - let parent_path = util::parent_path(&cli.input); - let directory = Dir::open_ambient_dir(parent_path, authority) - .with_context(|| format!("Failed to open directory: {parent_path:?}"))?; + let directory = util::parent_path(&cli.input); let name = cli .input .file_stem() @@ -816,7 +797,7 @@ fn digest_subcommand(cli: &DigestCli, cancel_signal: &AtomicBool) -> Result<()> .to_str() .ok_or_else(|| anyhow!("Invalid UTF-8: {:?}", cli.input))?; - let digest = compute_digest(&directory, name, cancel_signal)?; + let digest = compute_digest(directory, name, cancel_signal)?; println!("{}", hex::encode(digest)); diff --git a/avbroot/src/cli/cpio.rs b/avbroot/src/cli/cpio.rs index 35118a6..6dcb7e5 100644 --- a/avbroot/src/cli/cpio.rs +++ b/avbroot/src/cli/cpio.rs @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2023 Andrew Gunnerson +// SPDX-FileCopyrightText: 2023-2025 Andrew Gunnerson // SPDX-License-Identifier: GPL-3.0-only use std::{ @@ -11,7 +11,6 @@ use std::{ use anyhow::{Context, Result, anyhow}; use bstr::ByteSlice; -use cap_std::{ambient_authority, fs::Dir}; use clap::{Parser, Subcommand}; use num_traits::ToPrimitive; use serde::{Deserialize, Serialize}; @@ -91,17 +90,17 @@ fn write_info(path: &Path, info: &CpioInfo) -> Result<()> { /// Open reader to the corresponding file inside the tree if the entry is a /// regular file. Unsafe paths will result in an error. -fn open_tree_file(tree: &Dir, entry: &CpioEntry) -> Result, u32)>> { +fn open_tree_file(tree: &Path, entry: &CpioEntry) -> Result, u32)>> { if entry.file_type == CpioEntryType::Regular { - let path = entry + let sub_path = entry .path .as_bstr() .to_path() .with_context(|| format!("Invalid entry path: {:?}", entry.path.as_bstr()))?; + let path = util::path_join(tree, sub_path)?; - let mut reader = tree - .open(path) - .map(|f| BufReader::new(f.into_std())) + let mut reader = File::open(&path) + .map(BufReader::new) .with_context(|| format!("Failed to open for reading: {path:?}"))?; let file_size = reader @@ -122,21 +121,21 @@ fn open_tree_file(tree: &Dir, entry: &CpioEntry) -> Result Result>> { +fn create_tree_file(tree: &Path, entry: &CpioEntry) -> Result>> { if entry.file_type == CpioEntryType::Regular { - let path = entry + let sub_path = entry .path .as_bstr() .to_path() .with_context(|| format!("Invalid entry path: {:?}", entry.path.as_bstr()))?; - let parent = util::parent_path(path); + let path = util::path_join(tree, sub_path)?; + let parent = util::parent_path(&path); - tree.create_dir_all(parent) + fs::create_dir_all(parent) .with_context(|| format!("Failed to create directory: {parent:?}"))?; - let writer = tree - .create(path) - .map(|f| BufWriter::new(f.into_std())) + let writer = File::create(&path) + .map(BufWriter::new) .with_context(|| format!("Failed to open for writing: {path:?}"))?; Ok(Some(writer)) @@ -171,16 +170,13 @@ fn unpack_subcommand( display_format(cpio_cli, format); - let authority = ambient_authority(); - Dir::create_ambient_dir_all(&cli.output_tree, authority) + fs::create_dir_all(&cli.output_tree) .with_context(|| format!("Failed to create directory: {:?}", cli.output_tree))?; - let tree = Dir::open_ambient_dir(&cli.output_tree, authority) - .with_context(|| format!("Failed to open directory: {:?}", cli.output_tree))?; while let Some(entry) = reader.next_entry().context("Failed to read cpio entry")? { display_entry(cpio_cli, &entry); - if let Some(mut writer) = create_tree_file(&tree, &entry)? { + if let Some(mut writer) = create_tree_file(&cli.output_tree, &entry)? { let file_size = entry.data.size()?; stream::copy_n(&mut reader, &mut writer, file_size.into(), cancel_signal) @@ -209,12 +205,8 @@ fn pack_subcommand(cpio_cli: &CpioCli, cli: &PackCli, cancel_signal: &AtomicBool cpio::assign_inodes(&mut info.entries, true)?; - let authority = ambient_authority(); - let tree = Dir::open_ambient_dir(&cli.input_tree, authority) - .with_context(|| format!("Failed to open directory: {:?}", cli.input_tree))?; - for entry in &mut info.entries { - let out = open_tree_file(&tree, entry)?; + let out = open_tree_file(&cli.input_tree, entry)?; if let Some((_, file_size)) = &out { entry.data = CpioEntryData::Size(*file_size); diff --git a/avbroot/src/cli/lp.rs b/avbroot/src/cli/lp.rs index deec13b..0d62aa5 100644 --- a/avbroot/src/cli/lp.rs +++ b/avbroot/src/cli/lp.rs @@ -1,8 +1,7 @@ -// SPDX-FileCopyrightText: 2024 Andrew Gunnerson +// SPDX-FileCopyrightText: 2024-2025 Andrew Gunnerson // SPDX-License-Identifier: GPL-3.0-only use std::{ - ffi::OsStr, fs::{self, File}, io::{Seek, SeekFrom}, path::{Path, PathBuf}, @@ -10,7 +9,6 @@ use std::{ }; use anyhow::{Context, Result, bail}; -use cap_std::{ambient_authority, fs::Dir}; use clap::{CommandFactory, Parser, Subcommand}; use rayon::iter::{ IndexedParallelIterator, IntoParallelIterator, IntoParallelRefIterator, ParallelIterator, @@ -19,6 +17,7 @@ use rayon::iter::{ use crate::{ format::lp::{Extent, ExtentType, ImageType, Metadata, SECTOR_SIZE}, stream::{self, FromReader, PSeekFile, Reopen, ToWriter}, + util, }; fn open_lp_inputs(paths: &[impl AsRef]) -> Result<(Vec, Metadata)> { @@ -190,11 +189,8 @@ fn unpack_subcommand(lp_cli: &LpCli, cli: &UnpackCli, cancel_signal: &AtomicBool return Ok(()); } - let authority = ambient_authority(); - Dir::create_ambient_dir_all(&cli.output_images, authority) + fs::create_dir_all(&cli.output_images) .with_context(|| format!("Failed to create directory: {:?}", cli.output_images))?; - let directory = Dir::open_ambient_dir(&cli.output_images, authority) - .with_context(|| format!("Failed to open directory: {:?}", cli.output_images))?; let slot = &metadata.slots[0]; @@ -216,11 +212,11 @@ fn unpack_subcommand(lp_cli: &LpCli, cli: &UnpackCli, cancel_signal: &AtomicBool for partition in &group.partitions { // A partition name with unsafe characters fails during parsing. - let path = format!("{}.img", partition.name); + let path = + util::path_join_single(&cli.output_images, format!("{}.img", partition.name))?; - let file = directory - .create(&path) - .map(|f| PSeekFile::new(f.into_std())) + let file = File::create(&path) + .map(PSeekFile::new) .with_context(|| format!("Failed to open for writing: {path:?}"))?; file.set_len(partition.size()?) @@ -303,35 +299,21 @@ fn pack_subcommand(lp_cli: &LpCli, cli: &PackCli, cancel_signal: &AtomicBool) -> } } - for group in &slot.groups { - for partition in &group.partitions { - let name = &partition.name; - - if Path::new(name).file_name() != Some(OsStr::new(name)) { - bail!("Unsafe partition name: {name}"); - } - } - } - // Preopen all image input files. let mut paths = vec![]; let mut files = vec![]; if metadata.image_type == ImageType::Normal { - let authority = ambient_authority(); - let directory = Dir::open_ambient_dir(&cli.input_images, authority) - .with_context(|| format!("Failed to open directory: {:?}", cli.input_images))?; - for group in &mut slot.groups { let mut group_paths = vec![]; let mut group_files = vec![]; for partition in &mut group.partitions { - let path = format!("{}.img", partition.name); + let path = + util::path_join_single(&cli.input_images, format!("{}.img", partition.name))?; - let mut file = directory - .open(&path) - .map(|f| PSeekFile::new(f.into_std())) + let mut file = File::open(&path) + .map(PSeekFile::new) .with_context(|| format!("Failed to open for reading: {path:?}"))?; let size = file diff --git a/avbroot/src/cli/ota.rs b/avbroot/src/cli/ota.rs index f5cf29e..e926ec6 100644 --- a/avbroot/src/cli/ota.rs +++ b/avbroot/src/cli/ota.rs @@ -15,11 +15,9 @@ use std::{ use anyhow::{Context, Result, anyhow, bail}; use bitflags::bitflags; -use cap_std::{ambient_authority, fs::Dir}; -use cap_tempfile::TempDir; use clap::{ArgAction, Args, Parser, Subcommand, value_parser}; use rayon::{iter::IntoParallelRefIterator, prelude::ParallelIterator}; -use tempfile::NamedTempFile; +use tempfile::{NamedTempFile, TempDir}; use topological_sort::TopologicalSort; use tracing::{debug_span, error, info, warn}; use x509_cert::Certificate; @@ -1293,29 +1291,22 @@ fn patch_ota_zip( pub fn extract_payload( raw_reader: &PSeekFile, - directory: &Dir, + directory: &Path, payload_offset: u64, payload_size: u64, header: &PayloadHeader, images: &BTreeSet, cancel_signal: &AtomicBool, ) -> Result<()> { - for name in images { - if Path::new(name).file_name() != Some(OsStr::new(name)) { - bail!("Unsafe partition name: {name}"); - } - } - info!("Extracting from the payload: {}", util::join(images, ", ")); // Pre-open all output files. let output_files = images .iter() .map(|name| { - let path = format!("{name}.img"); - let file = directory - .create(&path) - .map(|f| PSeekFile::new(f.into_std())) + let path = util::path_join_single(directory, format!("{name}.img"))?; + let file = File::create(&path) + .map(PSeekFile::new) .with_context(|| format!("Failed to open for writing: {path:?}"))?; Ok((name.as_str(), file)) }) @@ -1345,7 +1336,7 @@ pub fn extract_payload( } fn verify_partition_hashes( - directory: &Dir, + directory: &Path, header: &PayloadHeader, images: &BTreeSet, cancel_signal: &AtomicBool, @@ -1365,9 +1356,8 @@ fn verify_partition_hashes( .and_then(|info| info.hash.as_ref()) .ok_or_else(|| anyhow!("Hash not found for partition: {name}"))?; - let path = format!("{name}.img"); - let file = directory - .open(&path) + let path = util::path_join_single(directory, format!("{name}.img"))?; + let file = File::open(&path) .with_context(|| format!("Failed to open for reading: {path:?}"))?; let mut writer = HashingWriter::new( @@ -1702,15 +1692,12 @@ pub fn extract_subcommand(cli: &ExtractCli, cancel_signal: &AtomicBool) -> Resul return Ok(()); } - let authority = ambient_authority(); - Dir::create_ambient_dir_all(&cli.directory, authority) + fs::create_dir_all(&cli.directory) .with_context(|| format!("Failed to create directory: {:?}", cli.directory))?; - let directory = Dir::open_ambient_dir(&cli.directory, authority) - .with_context(|| format!("Failed to open directory: {:?}", cli.directory))?; extract_payload( &raw_reader, - &directory, + &cli.directory, payload_offset, payload_size, &header, @@ -1742,9 +1729,9 @@ pub fn extract_subcommand(cli: &ExtractCli, cancel_signal: &AtomicBool) -> Resul .and_then(|p| p.device.first()) .ok_or_else(|| anyhow!("Device codename not found in OTA metadata"))?; - directory - .write(ANDROID_INFO, format!("require board={device}\n")) - .with_context(|| format!("Failed to write file: {ANDROID_INFO}"))?; + let android_info_path = util::path_join_single(&cli.directory, ANDROID_INFO)?; + fs::write(&android_info_path, format!("require board={device}\n")) + .with_context(|| format!("Failed to write file: {android_info_path:?}"))?; // Find out which images can be flashed with fastboot. The bootloader // (and potentially modem) partitions need to be flashed as a whole and @@ -1753,9 +1740,9 @@ pub fn extract_subcommand(cli: &ExtractCli, cancel_signal: &AtomicBool) -> Resul let mut flashable_images = BTreeSet::new(); for name in &unique_images { - let file = directory - .open(format!("{name}.img")) - .with_context(|| format!("Failed to open image for reading: {name}"))?; + let path = util::path_join_single(&cli.directory, format!("{name}.img"))?; + let file = File::open(&path) + .with_context(|| format!("Failed to open image for reading: {path:?}"))?; match avb::load_image(file) { Ok(_) => { @@ -1763,7 +1750,7 @@ pub fn extract_subcommand(cli: &ExtractCli, cancel_signal: &AtomicBool) -> Resul } // Treat images without AVB metadata as bootloader partitions. Err(avb::Error::InvalidHeaderMagic(_)) => continue, - Err(e) => return Err(e).with_context(|| format!("Failed to load image: {name}")), + Err(e) => return Err(e).with_context(|| format!("Failed to load image: {path:?}")), } } @@ -1851,9 +1838,9 @@ pub fn extract_subcommand(cli: &ExtractCli, cancel_signal: &AtomicBool) -> Resul fastboot_info.push_str("if-wipe erase userdata\n"); fastboot_info.push_str("if-wipe erase metadata\n"); - directory - .write(FASTBOOT_INFO, fastboot_info) - .with_context(|| format!("Failed to write file: {FASTBOOT_INFO}"))?; + let fastboot_info_path = util::path_join_single(&cli.directory, FASTBOOT_INFO)?; + fs::write(&fastboot_info_path, fastboot_info) + .with_context(|| format!("Failed to write file: {fastboot_info_path:?}"))?; } Ok(()) @@ -1934,8 +1921,7 @@ pub fn verify_subcommand(cli: &VerifyCli, cancel_signal: &AtomicBool) -> Result< info!("Extracting partition images to temporary directory"); - let authority = ambient_authority(); - let temp_dir = TempDir::new(authority).context("Failed to create temporary directory")?; + let temp_dir = TempDir::new().context("Failed to create temporary directory")?; let raw_reader = reader.into_inner(); let unique_images = header .manifest @@ -1947,7 +1933,7 @@ pub fn verify_subcommand(cli: &VerifyCli, cancel_signal: &AtomicBool) -> Result< extract_payload( &raw_reader, - &temp_dir, + temp_dir.path(), pf_payload.offset, pf_payload.size, &header, @@ -1957,7 +1943,8 @@ pub fn verify_subcommand(cli: &VerifyCli, cancel_signal: &AtomicBool) -> Result< info!("Verifying partition hashes"); - if let Err(e) = verify_partition_hashes(&temp_dir, &header, &unique_images, cancel_signal) { + if let Err(e) = verify_partition_hashes(temp_dir.path(), &header, &unique_images, cancel_signal) + { fail_later!("{e:?}"); } @@ -1977,7 +1964,7 @@ pub fn verify_subcommand(cli: &VerifyCli, cancel_signal: &AtomicBool) -> Result< let mut descriptors = HashMap::::new(); if let Err(e) = cli::avb::verify_headers( - &temp_dir, + temp_dir.path(), "vbmeta", public_key.as_ref(), &mut seen, @@ -1988,8 +1975,9 @@ pub fn verify_subcommand(cli: &VerifyCli, cancel_signal: &AtomicBool) -> Result< fail_later!("{e:?}"); } - if let Err(e) = cli::avb::verify_descriptors(&temp_dir, &descriptors, false, cancel_signal) - .context("Failed to verify images against AVB descriptors") + if let Err(e) = + cli::avb::verify_descriptors(temp_dir.path(), &descriptors, false, cancel_signal) + .context("Failed to verify images against AVB descriptors") { fail_later!("{e:?}"); } @@ -2003,11 +1991,10 @@ pub fn verify_subcommand(cli: &VerifyCli, cancel_signal: &AtomicBool) -> Result< .map(|(name, _)| name.as_str()) .collect::>(); let boot_images = boot::load_boot_images(&boot_image_names, |name| { - Ok(Box::new( - temp_dir - .open(format!("{name}.img")) - .map(|f| PSeekFile::new(f.into_std()))?, - )) + let path = util::path_join_single(temp_dir.path(), format!("{name}.img")) + .map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e))?; + + Ok(Box::new(File::open(path).map(PSeekFile::new)?)) }) .context("Failed to load all boot images")?; let targets = OtaCertPatcher::new(ota_cert.clone()) diff --git a/avbroot/src/cli/payload.rs b/avbroot/src/cli/payload.rs index afa568a..60ed329 100644 --- a/avbroot/src/cli/payload.rs +++ b/avbroot/src/cli/payload.rs @@ -1,9 +1,9 @@ -// SPDX-FileCopyrightText: 2024 Andrew Gunnerson +// SPDX-FileCopyrightText: 2024-2025 Andrew Gunnerson // SPDX-License-Identifier: GPL-3.0-only use std::{ collections::HashMap, - ffi::{OsStr, OsString}, + ffi::OsString, fs::{self, File}, io::{BufReader, BufWriter, Seek, SeekFrom}, path::{Path, PathBuf}, @@ -11,7 +11,6 @@ use std::{ }; use anyhow::{Context, Result, anyhow, bail}; -use cap_std::{ambient_authority, fs::Dir}; use clap::{Args, Parser, Subcommand}; use tracing::info; @@ -20,6 +19,7 @@ use crate::{ crypto::{self, PassphraseSource, RsaSigningKey}, format::payload::{PayloadHeader, PayloadWriter}, stream::{self, FromReader, PSeekFile}, + util, }; fn open_reader(path: &Path, allow_delta: bool) -> Result<(BufReader, PayloadHeader)> { @@ -113,15 +113,12 @@ fn unpack_subcommand( write_info(&cli.output_info, &header)?; - let authority = ambient_authority(); - Dir::create_ambient_dir_all(&cli.output_images, authority) + fs::create_dir_all(&cli.output_images) .with_context(|| format!("Failed to create directory: {:?}", cli.output_images))?; - let directory = Dir::open_ambient_dir(&cli.output_images, authority) - .with_context(|| format!("Failed to open directory: {:?}", cli.output_images))?; ota::extract_payload( &PSeekFile::new(reader.into_inner()), - &directory, + &cli.output_images, 0, payload_size, &header, @@ -147,28 +144,16 @@ fn pack_subcommand( let mut header = read_info(&cli.input_info)?; - let authority = ambient_authority(); - let directory = Dir::open_ambient_dir(&cli.input_images, authority) - .with_context(|| format!("Failed to open directory: {:?}", cli.input_images))?; - - for p in &header.manifest.partitions { - let name = &p.partition_name; - - if Path::new(name).file_name() != Some(OsStr::new(name)) { - bail!("Unsafe partition name: {name}"); - } - } - // Pre-open all of the image files. let input_files = header .manifest .partitions .iter() .map(|p| { - let path = format!("{}.img", p.partition_name); - let file = directory - .open(&path) - .map(|f| PSeekFile::new(f.into_std())) + let path = + util::path_join_single(&cli.input_images, format!("{}.img", p.partition_name))?; + let file = File::open(&path) + .map(PSeekFile::new) .with_context(|| format!("Failed to open file: {path:?}"))?; Ok((p.partition_name.clone(), file)) diff --git a/avbroot/src/patch/boot.rs b/avbroot/src/patch/boot.rs index ab5a615..f30f481 100644 --- a/avbroot/src/patch/boot.rs +++ b/avbroot/src/patch/boot.rs @@ -35,6 +35,7 @@ use crate::{ }, patch::otacert::{self, OtaCertBuildFlags}, stream::{self, FromReader, HashingWriter, ReadSeek, SectionReader, ToWriter, WriteSeek}, + util, }; #[derive(Debug, Error)] @@ -929,16 +930,17 @@ impl PrepatchedImagePatcher { return Ok(None); }; - let kmi_version = captures - .iter() - // Capture #0 is the entire match. - .skip(1) - .flatten() - .map(|c| c.as_bytes()) - // Our regex only matches ASCII bytes. - .map(|c| std::str::from_utf8(c).unwrap()) - .collect::>() - .join("-"); + let kmi_version = util::join( + captures + .iter() + // Capture #0 is the entire match. + .skip(1) + .flatten() + .map(|c| c.as_bytes()) + // Our regex only matches ASCII bytes. + .map(|c| std::str::from_utf8(c).unwrap()), + "-", + ); Ok(Some(kmi_version)) } diff --git a/avbroot/src/util.rs b/avbroot/src/util.rs index a08639d..e409da7 100644 --- a/avbroot/src/util.rs +++ b/avbroot/src/util.rs @@ -8,7 +8,7 @@ use std::{ ops::{ Bound, Range, RangeBounds, RangeFrom, RangeFull, RangeInclusive, RangeTo, RangeToInclusive, }, - path::Path, + path::{Component, Path, PathBuf}, }; use num_traits::{NumCast, PrimInt}; @@ -313,6 +313,67 @@ pub fn parent_path(path: &Path) -> &Path { Path::new(".") } +/// Path safety-related errors. +#[derive(Clone, Debug, Error)] +pub enum PathSafetyError { + #[error("Path contains multiple components: {0:?}")] + NotSingle(PathBuf), + #[error("Path contains '..': {0:?}")] + HasDotDot(PathBuf), +} + +/// Join `parent` with `child`, disallowing unsafe child paths. Absolute child +/// paths are converted into relative paths and `..` components result in an +/// error. +pub fn path_join( + parent: impl AsRef, + child: impl AsRef, +) -> Result { + fn inner(parent: &Path, child: &Path) -> Result { + let mut result = parent.to_owned(); + + for component in child.components() { + match component { + // Make absolute paths relative. + Component::Prefix(_) | Component::RootDir | Component::CurDir => continue, + // Unsafe path. We block this even if it wouldn't escape the parent. + Component::ParentDir => { + return Err(PathSafetyError::HasDotDot(child.to_path_buf())); + } + Component::Normal(os_str) => result.push(os_str), + } + } + + Ok(result) + } + + inner(parent.as_ref(), child.as_ref()) +} + +/// Ensure that the path has no directory components. +pub fn path_ensure_single(name: impl AsRef) -> Result<(), PathSafetyError> { + fn inner(name: &Path) -> Result<(), PathSafetyError> { + // Path::file_name() already checks for `.` and `..`. + if name.file_name() != Some(name.as_os_str()) { + return Err(PathSafetyError::NotSingle(name.to_path_buf())); + } + + Ok(()) + } + + inner(name.as_ref()) +} + +/// Like [`path_join`], but ensures that the child path contains no directory +/// components with [`path_ensure_single`]. +pub fn path_join_single( + parent: impl AsRef, + child: impl AsRef, +) -> Result { + path_ensure_single(child.as_ref())?; + path_join(parent.as_ref(), child.as_ref()) +} + /// Sort and merge overlapping intervals. pub fn merge_overlapping(sections: &[Range]) -> Vec> where @@ -379,6 +440,7 @@ where .is_ok() } +/// Join arbitrary displayable items with a separator. pub fn join(into_iter: impl IntoIterator, sep: &str) -> String { use std::fmt::Write; @@ -395,6 +457,7 @@ pub fn join(into_iter: impl IntoIterator, sep: &str) -> Str result } +/// Sort arbitrary sequence of sortable items. pub fn sort(iter: impl Iterator) -> Vec { let mut items = iter.collect::>(); items.sort(); diff --git a/e2e/src/main.rs b/e2e/src/main.rs index 697513e..9fe4826 100644 --- a/e2e/src/main.rs +++ b/e2e/src/main.rs @@ -47,6 +47,7 @@ use avbroot::{ }, }, stream::{self, CountingWriter, FromReader, HashingReader, PSeekFile, Reopen, ToWriter}, + util, }; use clap::Parser; use rsa::{BigUint, rand_core::OsRng, traits::PublicKeyParts}; @@ -1222,10 +1223,6 @@ fn test_subcommand(cli: &TestCli, cancel_signal: &AtomicBool) -> Result<()> { ]; for name in profiles { - if Path::new(name).file_name() != Some(OsStr::new(name)) { - bail!("Unsafe profile name: {name}"); - } - let profile = &config.profile[name]; for (zip_mode, hashes) in [ @@ -1235,7 +1232,7 @@ fn test_subcommand(cli: &TestCli, cancel_signal: &AtomicBool) -> Result<()> { let _span = info_span!("profile", name, %zip_mode).entered(); // Can't use NamedTempFile because avbroot does atomic replaces. - let mut profile_dir = work_dir.join(name); + let mut profile_dir = util::path_join_single(work_dir, name)?; profile_dir.push(zip_mode.to_string()); let out_original = profile_dir.join("ota.zip"); let out_magisk = profile_dir.join("ota_magisk.zip");