primitives: prevent null prevout in non-coinbase transactions
What the developer wrote
Authored by jrakibi
primitives: prevent null prevout in non-coinbase transactions
Non-coinbase transactions must not contain a null prevout (coinbase prevout)
Add this validation to the transaction decoder to reject txs
that violate this rule.
What changed, and why it matters
This commit adds a safety check in the Rust Bitcoin library's transaction decoder. It ensures that any transaction with more than one input cannot contain a 'coinbase-style' empty previous transaction reference. Such a reference is only allowed in the special coinbase transaction that creates new coins. Without this check, a malformed or malicious transaction could slip through and potentially cause incorrect behavior in software using this library to parse Bitcoin transactions.
Review whether the same validation is needed in other transaction construction or deserialization paths (e.g., direct from_bytes, consensus::Transaction, or psbt deserialization) to ensure consistent enforcement across the library.
Security signals we found
Consensus-rule enforcement added to transaction decoder
Rejection of null prevout in non-coinbase transactions
New error variant for malformed transaction input
Regression test from Bitcoin Core tx_invalid.json
Evidence from the diff
The patch modifies TransactionDecoder::end() in primitives/src/transaction.rs to reject decoded non-coinbase transactions that contain an OutPoint equal to OutPoint::COINBASE_PREVOUT (all-zeros with u32::MAX vout). The check is triggered when tx.inputs.len() > 1, matching Bitcoin consensus policy that a null prevout is only valid in a single-input coinbase transaction. A new error variant NullPrevoutInNonCoinbase(usize) is added, and a regression test using a Bitcoin Core tx_invalid.json vector is included.
Changed components
primitives/src/transaction.rsTransactionDecoder::end()TransactionDecoderErrorInnerTransactionDecoderError Display and Error implsInspect captured patch +31 / −1
diff --git a/primitives/src/transaction.rs b/primitives/src/transaction.rs
index a796e5ce..a092acad 100644
--- a/primitives/src/transaction.rs
+++ b/primitives/src/transaction.rs
@@ -538,7 +538,17 @@ impl Decoder for TransactionDecoder {
State::Outputs(..) => Err(E(Inner::EarlyEnd("outputs"))),
State::Witnesses(..) => Err(E(Inner::EarlyEnd("witnesses"))),
State::LockTime(..) => Err(E(Inner::EarlyEnd("locktime"))),
- State::Done(tx) => Ok(tx),
+ State::Done(tx) => {
+ // check for null prevout in non-coinbase txs
+ if tx.inputs.len() > 1 {
+ for (index, input) in tx.inputs.iter().enumerate() {
+ if input.previous_output == OutPoint::COINBASE_PREVOUT {
+ return Err(E(Inner::NullPrevoutInNonCoinbase(index)));
+ }
+ }
+ }
+ Ok(tx)
+ }
State::Errored => panic!("call to end() after decoder errored"),
}
}
@@ -640,6 +650,8 @@ enum TransactionDecoderErrorInner {
/// Attempt to call `end()` before the transaction was complete. Holds
/// a description of the current state.
EarlyEnd(&'static str),
+ /// Null prevout in non-coinbase transaction.
+ NullPrevoutInNonCoinbase(usize),
}
#[cfg(feature = "alloc")]
@@ -692,6 +704,7 @@ impl fmt::Display for TransactionDecoderError {
E::NoWitnesses => write!(f, "non-empty Segwit transaction with no witnesses"),
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),
}
}
}
@@ -711,6 +724,7 @@ impl std::error::Error for TransactionDecoderError {
E::NoWitnesses => None,
E::LockTime(ref e) => Some(e),
E::EarlyEnd(_) => None,
+ E::NullPrevoutInNonCoinbase(_) => None,
}
}
}
@@ -2192,4 +2206,20 @@ mod tests {
assert_eq!(original_tx, decoded_tx);
}
+
+ #[test]
+ #[cfg(all(feature = "alloc", feature = "hex"))]
+ fn reject_null_prevout_in_non_coinbase_transaction() {
+ // Test vector taken from Bitcoin Core tx_invalid.json
+ // https://github.com/bitcoin/bitcoin/blob/master/src/test/data/tx_invalid.json#L64
+ // "Null txin, but without being a coinbase (because there are two inputs)"
+ let tx_bytes = hex!("01000000020000000000000000000000000000000000000000000000000000000000000000ffffffff00ffffffff00010000000000000000000000000000000000000000000000000000000000000000000000ffffffff010000000000000000015100000000");
+
+ let mut decoder = Transaction::decoder();
+ let mut slice = tx_bytes.as_slice();
+ decoder.push_bytes(&mut slice).unwrap();
+ let err = decoder.end().expect_err("null prevout in non-coinbase tx should be rejected");
+
+ assert_eq!(err, TransactionDecoderError(TransactionDecoderErrorInner::NullPrevoutInNonCoinbase(0)));
+ }
}
Why this scored 60/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.