primitives: reject transactions with invalid coinbase scriptSig length
What the developer wrote
Authored by jrakibi
primitives: reject transactions with invalid coinbase scriptSig length
Coinbase input scriptSig must be between 2 and 100 bytes in length.
Add this validation rule to transaction decoder
What changed, and why it matters
This commit adds a missing validation rule to the library's Bitcoin transaction decoder: a coinbase transaction (the special first transaction in a block that creates new coins) must have an input script between 2 and 100 bytes long. Before this change, the library would accept shorter or longer coinbase scripts, which violates Bitcoin consensus rules and could let malformed transactions slip through. The fix rejects them with clear errors and includes tests for the boundary cases.
Review whether any other transaction decoders or deserialization paths in the crate bypass this decoder and could still accept invalid coinbase lengths. Re-enable or update the ignored block_decode test promptly. Consider whether this rule should also be enforced at construction time for programmatically built coinbase transactions, not only during decoding.
Security signals we found
Consensus-rule validation gap closed in transaction decoder
New explicit error variants for malformed coinbase scriptSig lengths
Test vectors borrowed from Bitcoin Core tx_invalid.json
Existing test marked ignored due to previously accepted invalid coinbase
Evidence from the diff
The patch enforces BIP/consensus rule that a coinbase input’s scriptSig must be 2-100 bytes. It inserts a length check in TransactionDecoder::end() after decoding, returning new errors CoinbaseScriptSigTooSmall and CoinbaseScriptSigTooLarge. Unit tests are added covering 1-byte (too small), 101-byte (too large), 2-byte (minimum valid), and 100-byte (maximum valid) coinbase scripts, using vectors derived from Bitcoin Core’s tx_invalid.json. An existing block_decode test is marked #[ignore] because it used an invalid coinbase script length and will be fixed separately.
Changed components
primitives/src/transaction.rs TransactionDecoderprimitives/src/block.rs block_decode testInspect captured patch +79 / −0
diff --git a/primitives/src/block.rs b/primitives/src/block.rs
index 8b9f212a..38203577 100644
--- a/primitives/src/block.rs
+++ b/primitives/src/block.rs
@@ -1162,6 +1162,7 @@ mod tests {
}
#[test]
+ #[ignore] // bad test; will be fixed in next commit
#[cfg(feature = "alloc")]
fn block_decode() {
// Make a simple block, encode then decode. Verify equivalence.
diff --git a/primitives/src/transaction.rs b/primitives/src/transaction.rs
index a092acad..e69baa09 100644
--- a/primitives/src/transaction.rs
+++ b/primitives/src/transaction.rs
@@ -547,6 +547,16 @@ impl Decoder for TransactionDecoder {
}
}
}
+ // check coinbase scriptSig length (must be 2-100 bytes)
+ if tx.is_coinbase() {
+ let len = tx.inputs[0].script_sig.len();
+ if len < 2 {
+ return Err(E(Inner::CoinbaseScriptSigTooSmall(len)));
+ }
+ if len > 100 {
+ return Err(E(Inner::CoinbaseScriptSigTooLarge(len)));
+ }
+ }
Ok(tx)
}
State::Errored => panic!("call to end() after decoder errored"),
@@ -652,6 +662,10 @@ enum TransactionDecoderErrorInner {
EarlyEnd(&'static str),
/// Null prevout in non-coinbase transaction.
NullPrevoutInNonCoinbase(usize),
+ /// Coinbase scriptSig too small (must be at least 2 bytes).
+ CoinbaseScriptSigTooSmall(usize),
+ /// Coinbase scriptSig is too large (must be at most 100 bytes).
+ CoinbaseScriptSigTooLarge(usize),
}
#[cfg(feature = "alloc")]
@@ -705,6 +719,8 @@ impl fmt::Display for TransactionDecoderError {
E::LockTime(ref e) => write_err!(f, "transaction decoder error"; e),
E::EarlyEnd(s) => write!(f, "early end of transaction (still decoding {})", s),
E::NullPrevoutInNonCoinbase(index) => write!(f, "null prevout in non-coinbase transaction at input {}", index),
+ E::CoinbaseScriptSigTooSmall(len) => write!(f, "coinbase scriptSig too small: {} bytes (min 2)", len),
+ E::CoinbaseScriptSigTooLarge(len) => write!(f, "coinbase scriptSig too large: {} bytes (max 100)", len),
}
}
}
@@ -725,6 +741,8 @@ impl std::error::Error for TransactionDecoderError {
E::LockTime(ref e) => Some(e),
E::EarlyEnd(_) => None,
E::NullPrevoutInNonCoinbase(_) => None,
+ E::CoinbaseScriptSigTooSmall(_) => None,
+ E::CoinbaseScriptSigTooLarge(_) => None,
}
}
}
@@ -2222,4 +2240,64 @@ mod tests {
assert_eq!(err, TransactionDecoderError(TransactionDecoderErrorInner::NullPrevoutInNonCoinbase(0)));
}
+
+ #[test]
+ #[cfg(all(feature = "alloc", feature = "hex"))]
+ fn reject_coinbase_scriptsig_too_small() {
+ // Test vector taken from Bitcoin Core tx_invalid.json
+ // https://github.com/bitcoin/bitcoin/blob/master/src/test/data/tx_invalid.json#L57
+ // "Coinbase of size 1"
+ let tx_bytes = hex!("01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff0151ffffffff010000000000000000015100000000");
+
+ let mut decoder = Transaction::decoder();
+ let mut slice = tx_bytes.as_slice();
+ decoder.push_bytes(&mut slice).unwrap();
+ let err = decoder.end().expect_err("coinbase with 1-byte scriptSig should be rejected");
+
+ assert_eq!(err, TransactionDecoderError(TransactionDecoderErrorInner::CoinbaseScriptSigTooSmall(1)));
+ }
+
+ #[test]
+ #[cfg(all(feature = "alloc", feature = "hex"))]
+ fn reject_coinbase_scriptsig_too_large() {
+ // Test vector taken from Bitcoin Core tx_invalid.json:
+ // https://github.com/bitcoin/bitcoin/blob/master/src/test/data/tx_invalid.json#L62
+ // "Coinbase of size 101"
+ let tx_bytes = hex!("01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff655151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151ffffffff010000000000000000015100000000");
+
+ let mut decoder = Transaction::decoder();
+ let mut slice = tx_bytes.as_slice();
+ decoder.push_bytes(&mut slice).unwrap();
+ let err = decoder.end().expect_err("coinbase with 101-byte scriptSig should be rejected");
+
+ assert_eq!(err, TransactionDecoderError(TransactionDecoderErrorInner::CoinbaseScriptSigTooLarge(101)));
+ }
+
+ #[test]
+ #[cfg(all(feature = "alloc", feature = "hex"))]
+ fn accept_coinbase_scriptsig_min_valid() {
+ // boundary test: 2 bytes is the minimum valid length
+ let tx_bytes = hex!("01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff025151ffffffff010000000000000000015100000000");
+
+ let mut decoder = Transaction::decoder();
+ let mut slice = tx_bytes.as_slice();
+ decoder.push_bytes(&mut slice).unwrap();
+ let tx = decoder.end().expect("coinbase with 2-byte scriptSig should be accepted");
+
+ assert_eq!(tx.inputs[0].script_sig.len(), 2);
+ }
+
+ #[test]
+ #[cfg(all(feature = "alloc", feature = "hex"))]
+ fn accept_coinbase_scriptsig_max_valid() {
+ // boundary test: 100 bytes is the maximum valid length
+ let tx_bytes = hex!("01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff6451515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151ffffffff010000000000000000015100000000");
+
+ let mut decoder = Transaction::decoder();
+ let mut slice = tx_bytes.as_slice();
+ decoder.push_bytes(&mut slice).unwrap();
+ let tx = decoder.end().expect("coinbase with 100-byte scriptSig should be accepted");
+
+ assert_eq!(tx.inputs[0].script_sig.len(), 100);
+ }
}
Why this scored 52/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.