diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 000000000..50ae3b342 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2025-05-14 - Arithmetic Hardening in Entry Parsing and Splitting +**Vulnerability:** Potential panics from integer overflows during archive entry parsing and multi-part archive splitting. +**Learning:** Cumulative size calculations (like `compressed_size` or total entry length) and counters (like archive part numbers) in archive logic are vulnerable to overflows when processing large or malformed archives, especially on 32-bit platforms where `usize::MAX` is smaller. +**Prevention:** Use `saturating_add` for size and length calculations to safely maintain upper bounds without crashing, and `checked_add` for structural fields like sequence numbers where an explicit error is preferable to wrap-around or saturation. diff --git a/cli/src/command/core.rs b/cli/src/command/core.rs index 28e0add9b..32e6937d8 100644 --- a/cli/src/command/core.rs +++ b/cli/src/command/core.rs @@ -1812,27 +1812,29 @@ where ) .into()); } - let mut part_num = 1; + let mut part_num: usize = 1; let mut writer = Archive::write_header(initial_writer)?; // NOTE: max_file_size - (PNA_HEADER + AHED + ANXT + AEND) let max_file_size = max_file_size - SPLIT_ARCHIVE_OVERHEAD_BYTES; - let mut written_entry_size = 0; + let mut written_entry_size: usize = 0; for entry in entries { let p = EntryPart::from(entry?); let parts = split_to_parts( p.as_ref(), - max_file_size - written_entry_size, + max_file_size.saturating_sub(written_entry_size), max_file_size, )?; for part in parts { - if written_entry_size + part.bytes_len() > max_file_size { - part_num += 1; + if written_entry_size.saturating_add(part.bytes_len()) > max_file_size { + part_num = part_num.checked_add(1).ok_or_else(|| { + io::Error::new(io::ErrorKind::InvalidData, "archive part number overflow") + })?; let file = get_next_writer(part_num)?; writer = writer.split_to_next_archive(file)?; written_entry_size = 0; } - written_entry_size += writer.add_entry_part(part)?; + written_entry_size = written_entry_size.saturating_add(writer.add_entry_part(part)?); } } writer.finalize()?; diff --git a/lib/src/entry.rs b/lib/src/entry.rs index e36f24aa3..c0e261fb7 100644 --- a/lib/src/entry.rs +++ b/lib/src/entry.rs @@ -633,7 +633,7 @@ where ), )); } - let mut compressed_size = 0; + let mut compressed_size: usize = 0; let mut extra = vec![]; let mut data = vec![]; let mut xattrs = vec![]; @@ -657,7 +657,7 @@ where ); } ChunkType::FDAT => { - compressed_size += chunk.data().len(); + compressed_size = compressed_size.saturating_add(chunk.data().len()); data.push(chunk.data); } ChunkType::fSIZ => size = Some(u128_from_be_bytes_last(chunk.data())), @@ -1132,7 +1132,9 @@ where /// Returns the total length of this entry part in bytes. #[inline] pub fn bytes_len(&self) -> usize { - self.0.iter().map(|chunk| chunk.bytes_len()).sum() + self.0 + .iter() + .fold(0, |acc, chunk| acc.saturating_add(chunk.bytes_len())) } /// Get reference. @@ -1156,11 +1158,18 @@ impl EntryPart<&[u8]> { } let mut remaining = VecDeque::from(self.0); let mut first = Vec::new(); - let mut total_size = 0; + let mut total_size: usize = 0; while let Some(chunk) = remaining.pop_front() { // NOTE: If over max size, restore to the remaining chunk - if max_bytes_len < total_size + chunk.bytes_len() { - if chunk.is_stream_chunk() && total_size + MIN_CHUNK_BYTES_SIZE < max_bytes_len { + if total_size + .checked_add(chunk.bytes_len()) + .is_none_or(|sum| max_bytes_len < sum) + { + if chunk.is_stream_chunk() + && total_size + .checked_add(MIN_CHUNK_BYTES_SIZE) + .is_some_and(|sum| sum < max_bytes_len) + { let available_bytes_len = max_bytes_len - total_size; let chunk_split_index = available_bytes_len - MIN_CHUNK_BYTES_SIZE; let (x, y) = chunk_data_split(chunk.ty, chunk.data, chunk_split_index); @@ -1173,7 +1182,7 @@ impl EntryPart<&[u8]> { } break; } - total_size += chunk.bytes_len(); + total_size = total_size.saturating_add(chunk.bytes_len()); first.push(chunk); } if first.is_empty() {