What the developer wrote
Authored by Tobin C. Harding
Only return valid data in sig slice
The `SerializedSignature` types have an internal buffer and we track
the length of valid data in the buffer. However in `AsRef` and friends
we are returning a slice of the whole buffer. This is confusing and a
potential footgun (eg `sig.as_ref()l.en()` isn't be meaningful).
Make the impls that get a byte slice reference only return valid data.
Done for both `ecdsa` and `taproot` type.
What changed, and why it matters
This commit fixes a bug in how serialized Bitcoin signatures expose their byte content. Previously, code asking for the signature bytes could receive the entire internal buffer, including leftover or uninitialized bytes beyond the actual signature length. Now only the valid, used portion is returned. This prevents accidental misuse where someone might hash, encode, or transmit extra garbage bytes as if they were part of the signature.
Review all call sites that rely on `AsRef<[u8]>` or `Borrow<[u8]>` for `SerializedSignature`, and consider whether `Deref` should also be restricted to valid data for consistency. Add regression tests verifying that `as_ref().len()` equals `len()` and that no trailing buffer bytes leak out.
Security signals we found
information disclosure: full internal buffer exposed through AsRef/Borrow
data integrity: extra bytes could be included in downstream hashing/encoding/serialization
footgun: length of slice no longer matches semantic signature length
partial fix: Deref still returns full buffer
Evidence from the diff
The SerializedSignature types in bitcoin/src/crypto/ecdsa.rs and bitcoin/src/crypto/taproot.rs store signature bytes in a fixed-size internal buffer (self.data) while tracking the real length in self.len. The AsRef<[u8]> and Borrow<[u8]> implementations previously returned a slice of the whole buffer via self (which dereferences to &self.data[..] through Deref). The patch changes them to return &self.data[..self.len], so callers only see the initialized/valid bytes. Deref itself is not changed in the diff, so *serialized_sig may still expose the full buffer; the commit is partial in that sense.
Changed components
bitcoin/src/crypto/ecdsa.rs: SerializedSignaturebitcoin/src/crypto/taproot.rs: SerializedSignatureInspect captured patch +4 / −4
diff --git a/bitcoin/src/crypto/ecdsa.rs b/bitcoin/src/crypto/ecdsa.rs
index 8610d1c1..be82535f 100644
--- a/bitcoin/src/crypto/ecdsa.rs
+++ b/bitcoin/src/crypto/ecdsa.rs
@@ -192,7 +192,7 @@ impl core::hash::Hash for SerializedSignature {
impl AsRef<[u8]> for SerializedSignature {
#[inline]
- fn as_ref(&self) -> &[u8] { self }
+ fn as_ref(&self) -> &[u8] { &self.data[..self.len] }
}
impl AsRef<PushBytes> for SerializedSignature {
@@ -202,7 +202,7 @@ impl AsRef<PushBytes> for SerializedSignature {
impl core::borrow::Borrow<[u8]> for SerializedSignature {
#[inline]
- fn borrow(&self) -> &[u8] { self }
+ fn borrow(&self) -> &[u8] { &self.data[..self.len] }
}
impl core::ops::Deref for SerializedSignature {
diff --git a/bitcoin/src/crypto/taproot.rs b/bitcoin/src/crypto/taproot.rs
index 17ce395e..1ce868e7 100644
--- a/bitcoin/src/crypto/taproot.rs
+++ b/bitcoin/src/crypto/taproot.rs
@@ -209,12 +209,12 @@ impl core::hash::Hash for SerializedSignature {
impl AsRef<[u8]> for SerializedSignature {
#[inline]
- fn as_ref(&self) -> &[u8] { self }
+ fn as_ref(&self) -> &[u8] { &self.data[..self.len] }
}
impl Borrow<[u8]> for SerializedSignature {
#[inline]
- fn borrow(&self) -> &[u8] { self }
+ fn borrow(&self) -> &[u8] { &self.data[..self.len] }
}
impl ops::Deref for SerializedSignature {
Why this scored 41/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.