Fix ScriptBuf::push_slice() handling of 0x00
What the developer wrote
Authored by Nadav Ivgi
Fix ScriptBuf::push_slice() handling of 0x00
And delegate Builder::push_slice() to use it.
What changed, and why it matters
This commit fixes how the library builds Bitcoin script instructions when pushing a single zero byte (0x00) onto the stack. Previously, ScriptBuf::push_slice() emitted an invalid opcode (OP_PUSHBYTES_0, which does not exist in Bitcoin) for a 0x00 byte, while Builder::push_slice() had its own separate logic that already handled 0x00 correctly by falling through to a normal non-minimal push. The patch makes Builder delegate to ScriptBuf so both behave the same way, and changes ScriptBuf to emit a valid push instruction for 0x00. A script containing the invalid opcode would be rejected by the Bitcoin network, so this is a correctness/security bug in transaction/script construction.
Review any code that uses ScriptBuf::push_slice() to push a single 0x00 byte, especially in transaction or script generation paths, and verify that the resulting scripts are now valid. Consider adding regression tests for 0x00 and other boundary single-byte pushes. Upgrade to the patched version.
Security signals we found
Script construction correctness bug
Invalid Bitcoin opcode emitted for 0x00 byte push
Potential transaction/script invalidity if generated script is used
Unification of duplicated push_slice logic between Builder and ScriptBuf
No explicit CVE or vendor security disclosure in commit
Evidence from the diff
In owned.rs, push_slice() previously matched bytes[0] == 0 with OP_PUSHBYTES_0. There is no such opcode in Bitcoin (the valid single-byte push opcodes are OP_0/OP_FALSE = 0x00 and OP_1 through OP_16 = 0x51-0x60). The 0x00 opcode is OP_0, which pushes an empty array, not a one-byte array of 0x00. The old code therefore produced an invalid script when asked to push a slice containing a single 0x00 byte. Builder::push_slice() in builder.rs had a different implementation that did not special-case 0x00, so it produced a correct non-minimal push. The patch unifies the two by making Builder call ScriptBuf::push_slice, and changes ScriptBuf to fall through to push_slice_non_minimal for any single byte other than 0x81 or 1..=16, including 0x00. This ensures a valid, non-minimal push of the byte 0x00.
Changed components
bitcoin/src/blockdata/script/owned.rsbitcoin/src/blockdata/script/builder.rsScriptBuf::push_sliceBuilder::push_sliceInspect captured patch +6 / −14
diff --git a/bitcoin/src/blockdata/script/builder.rs b/bitcoin/src/blockdata/script/builder.rs
index 2935dd46..ac246e4f 100644
--- a/bitcoin/src/blockdata/script/builder.rs
+++ b/bitcoin/src/blockdata/script/builder.rs
@@ -74,17 +74,10 @@ impl<T> Builder<T> {
}
/// Adds instructions to push some arbitrary data onto the stack.
- pub fn push_slice<D: AsRef<PushBytes>>(self, data: D) -> Self {
- let bytes = data.as_ref().as_bytes();
- if bytes.len() == 1 {
- match bytes[0] {
- 0x81 => self.push_opcode(OP_1NEGATE),
- 1..=16 => self.push_opcode(Opcode::from(bytes[0] + (OP_1.to_u8() - 1))),
- _ => self.push_slice_non_minimal(data),
- }
- } else {
- self.push_slice_non_minimal(data)
- }
+ pub fn push_slice<D: AsRef<PushBytes>>(mut self, data: D) -> Self {
+ self.0.push_slice(data);
+ self.1 = None;
+ self
}
/// Adds instructions to push some arbitrary data onto the stack without minimality.
diff --git a/bitcoin/src/blockdata/script/owned.rs b/bitcoin/src/blockdata/script/owned.rs
index 9cbd0f7b..1d8861c3 100644
--- a/bitcoin/src/blockdata/script/owned.rs
+++ b/bitcoin/src/blockdata/script/owned.rs
@@ -76,12 +76,11 @@ internal_macros::define_extension_trait! {
/// Adds instructions to push some arbitrary data onto the stack.
fn push_slice<D: AsRef<PushBytes>>(&mut self, data: D) {
let bytes = data.as_ref().as_bytes();
- if bytes.len() == 1 && (bytes[0] == 0x81 || bytes[0] <= 16) {
+ if bytes.len() == 1 {
match bytes[0] {
0x81 => { self.push_opcode(OP_1NEGATE); },
- 0 => { self.push_opcode(OP_PUSHBYTES_0); },
1..=16 => { self.push_opcode(Opcode::from(bytes[0] + (OP_1.to_u8() - 1))); },
- _ => {}, // unreachable arm
+ _ => { self.push_slice_non_minimal(data); },
}
} else {
self.push_slice_non_minimal(data);
Why this scored 63/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.