primitives: Add private cast_to_usize_if_valid function
What the developer wrote
Authored by Tobin C. Harding
primitives: Add private cast_to_usize_if_valid function
I have a bit of confusion around this 4,000,000 value so add a
_private_ function to the `witness` module and call it in two places
to remove casts of `u32` to `usize` that would likely never be hit
anyways.
I also remove the issue link, the issue is till open but its not
useful now to have a link to it in code.
What changed, and why it matters
This commit adds a small safety check in the code that reads Bitcoin transaction witness data. Previously, the code directly converted a length value from a u64 to a usize, which on very small systems (16-bit) could silently truncate a large number into a small one, potentially causing incorrect data reads. The new helper function rejects impossibly large lengths and safely checks whether the value fits in a usize. It is a defensive hardening change, not a fix for a confirmed exploitable bug.
Treat as a low-risk defensive hardening patch. Reviewers should confirm that `MAX_VEC_SIZE = 4_000_000` is consistent with Bitcoin Core's `MAX_SIZE` and that returning `None`/`None` from `get()` and `Iter::next()` on oversized lengths does not break any downstream invariants. No urgent security response is indicated absent evidence of an exploitable truncation path on supported targets.
Security signals we found
Replaces unchecked `u64 as usize` casts with a bounds-checked conversion
Adds an upper-bound sanity check (MAX_VEC_SIZE = 4,000,000) on decoded compact-size length prefixes
Uses `usize::try_from` to avoid silent integer truncation on 16-bit platforms
Removes a code comment referencing an open issue, indicating the previous assumption was considered unreliable
Defensive hardening in witness parsing, a consensus-critical data structure
Evidence from the diff
The patch introduces a private helper cast_to_usize_if_valid(n: u64) -> Option<usize> in primitives/src/witness.rs. It replaces two direct element_len as usize casts in Witness::get() and Iter::next() with calls that first verify the decoded compact-size length is at most MAX_VEC_SIZE (4,000,000 bytes) and then use usize::try_from(n).ok(). On 16-bit targets this prevents silent truncation of a large u64 into a small usize; on 32/64-bit targets it is largely a no-op because 4,000,000 fits comfortably in usize. The commit also removes an inline comment linking to issue #3264. The function is private and only used internally for witness element length decoding.
Changed components
primitives/src/witness.rsWitness::get()Iter<'a>::next()Witness element length decoding via compact_size::decode_uncheckedInspect captured patch +23 / −6
diff --git a/primitives/src/witness.rs b/primitives/src/witness.rs
index ac830497..375d18d1 100644
--- a/primitives/src/witness.rs
+++ b/primitives/src/witness.rs
@@ -230,9 +230,7 @@ impl Witness {
let mut slice = &self.content[pos..]; // Start of element.
let element_len = compact_size::decode_unchecked(&mut slice);
- // Compact size should always fit into a u32 because of `MAX_SIZE` in Core.
- // ref: https://github.com/rust-bitcoin/rust-bitcoin/issues/3264
- let end = element_len as usize;
+ let end = cast_to_usize_if_valid(element_len)?;
Some(&slice[..end])
}
@@ -562,9 +560,7 @@ impl<'a> Iterator for Iter<'a> {
let index = decode_cursor(self.inner, self.indices_start, self.current_index)?;
let mut slice = &self.inner[index..]; // Start of element.
let element_len = compact_size::decode_unchecked(&mut slice);
- // Compact size should always fit into a u32 because of `MAX_SIZE` in Core.
- // ref: https://github.com/rust-bitcoin/rust-bitcoin/issues/3264
- let end = element_len as usize;
+ let end = cast_to_usize_if_valid(element_len)?;
self.current_index += 1;
Some(&slice[..end])
}
@@ -809,6 +805,27 @@ impl<'a> Arbitrary<'a> for Witness {
}
}
+/// Cast a decoded length prefix to a `usize`.
+///
+/// This function is basically just defensive. For all sane use cases the length prefix should be
+/// less than `MAX_VEC_SIZE` (on a 32-bit machine). If the value is bigger that `u16::MAX` and we
+/// are on a 16-bit machine you'll likely hit an error later anyway, better to just check it now.
+///
+/// # 16-bits
+///
+/// The compact size may be bigger than what can be represented in a `usize` on a 16-bit machine but
+/// this shouldn't happen if we created the witness because one would get an OOM error before that.
+fn cast_to_usize_if_valid(n: u64) -> Option<usize> {
+ /// Maximum size, in bytes, of a vector we are allowed to decode.
+ const MAX_VEC_SIZE: u64 = 4_000_000;
+
+ if n > MAX_VEC_SIZE {
+ return None;
+ }
+
+ usize::try_from(n).ok()
+}
+
#[cfg(test)]
mod test {
#[cfg(feature = "alloc")]
Why this scored 32/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.