primitives: reject transaction with duplicate inputs
What the developer wrote
Authored by jrakibi
primitives: reject transaction with duplicate inputs
As part of adding validation rules to transactions, this check rejects
transactions with duplicate inputs during the decoding stage
Also updated a test case in the encode/decode roundtrip, the test was
using duplicated inputs which causes it to fail with the current
implementation.
Addresses part of https://github.com/rust-bitcoin/rust-bitcoin/issues/5383
Related: CVE-2018-17144
What changed, and why it matters
This commit adds a safety check in the rust-bitcoin library so that when a Bitcoin transaction is being decoded, it is rejected if it tries to spend the same coin twice within the same transaction. This prevents a known bug class (CVE-2018-17144) where duplicate inputs could cause nodes to crash or accept invalid transactions. The change also updates existing tests that accidentally used duplicate inputs.
Review whether duplicate-input rejection should also apply to transaction construction APIs (not only decoding), and confirm the error is propagated correctly by downstream consumers. Consider adding fuzzing or additional test vectors for edge cases such as many inputs or coinbase transactions.
Security signals we found
Adds duplicate-input validation during transaction decoding
Explicitly references CVE-2018-17144 in commit message and code comment
Adds negative test using Bitcoin Core tx_invalid.json vector
Updates existing tests that previously used duplicate inputs
Evidence from the diff
The patch modifies primitives/src/transaction.rs to reject transactions containing duplicate previous outputs (inputs) during TransactionDecoder::end(). It collects all input outpoints, sorts them, and checks adjacent pairs for equality, returning a new DuplicateInput error if found. A test vector from Bitcoin Core’s tx_invalid.json is added to verify rejection. Existing roundtrip tests are adjusted to use distinct inputs. The commit message explicitly references CVE-2018-17144 and issue #5383.
Changed components
primitives/src/transaction.rsTransactionDecoderTransactionDecoderErrorInnerInspect captured patch +50 / −2
diff --git a/primitives/src/transaction.rs b/primitives/src/transaction.rs
index 60fddd81..f8897207 100644
--- a/primitives/src/transaction.rs
+++ b/primitives/src/transaction.rs
@@ -606,6 +606,14 @@ impl Decoder for TransactionDecoder {
return Err(E(Inner::CoinbaseScriptSigTooLarge(len)));
}
}
+ // check for duplicate inputs (CVE-2018-17144).
+ let mut outpoints: Vec<_> = tx.inputs.iter().map(|i| i.previous_output).collect();
+ outpoints.sort_unstable();
+ for pair in outpoints.windows(2) {
+ if pair[0] == pair[1] {
+ return Err(E(Inner::DuplicateInput(pair[0])));
+ }
+ }
Ok(tx)
}
State::Errored => panic!("call to end() after decoder errored"),
@@ -715,6 +723,8 @@ enum TransactionDecoderErrorInner {
CoinbaseScriptSigTooSmall(usize),
/// Coinbase scriptSig is too large (must be at most 100 bytes).
CoinbaseScriptSigTooLarge(usize),
+ /// Transaction has duplicate inputs (this check prevents CVE-2018-17144 ).
+ DuplicateInput(OutPoint),
}
#[cfg(feature = "alloc")]
@@ -773,6 +783,8 @@ impl fmt::Display for TransactionDecoderError {
write!(f, "coinbase scriptSig too small: {} bytes (min 2)", len),
E::CoinbaseScriptSigTooLarge(len) =>
write!(f, "coinbase scriptSig too large: {} bytes (max 100)", len),
+ E::DuplicateInput(ref outpoint) =>
+ write!(f, "duplicate input: {:?}:{}", outpoint.txid, outpoint.vout),
}
}
}
@@ -795,6 +807,7 @@ impl std::error::Error for TransactionDecoderError {
E::NullPrevoutInNonCoinbase(_) => None,
E::CoinbaseScriptSigTooSmall(_) => None,
E::CoinbaseScriptSigTooLarge(_) => None,
+ E::DuplicateInput(_) => None,
}
}
}
@@ -1608,10 +1621,15 @@ mod tests {
#[cfg(feature = "alloc")]
#[cfg(feature = "hex")]
fn transaction_encode_decode_roundtrip() {
+ // Create two different inputs to avoid duplicate input rejection
+ let tx_in_1 = segwit_tx_in();
+ let mut tx_in_2 = segwit_tx_in();
+ tx_in_2.previous_output.vout = 2;
+
let tx = Transaction {
version: Version::TWO,
lock_time: absolute::LockTime::ZERO,
- inputs: vec![segwit_tx_in(), segwit_tx_in()],
+ inputs: vec![tx_in_1, tx_in_2],
outputs: vec![tx_out(), tx_out()],
};
@@ -1721,11 +1739,16 @@ mod tests {
#[test]
#[cfg(feature = "hex")]
fn transaction_from_hex_str_round_trip() {
+ // Create two different inputs to avoid duplicate input rejection
+ let tx_in_1 = segwit_tx_in();
+ let mut tx_in_2 = segwit_tx_in();
+ tx_in_2.previous_output.vout = 2;
+
// Create a transaction and convert it to a hex string
let tx = Transaction {
version: Version::TWO,
lock_time: absolute::LockTime::ZERO,
- inputs: vec![segwit_tx_in(), segwit_tx_in()],
+ inputs: vec![tx_in_1, tx_in_2],
outputs: vec![tx_out(), tx_out()],
};
@@ -2452,4 +2475,29 @@ mod tests {
assert_eq!(tx.inputs[0].script_sig.len(), 100);
}
+
+ #[test]
+ #[cfg(all(feature = "alloc", feature = "hex"))]
+ fn reject_duplicate_inputs() {
+ // Test vector from Bitcoin Core tx_invalid.json:
+ // https://github.com/bitcoin/bitcoin/blob/master/src/test/data/tx_invalid.json#L50
+ // Transaction has two inputs both spending the same outpoint
+ let tx_bytes = hex!("01000000020001000000000000000000000000000000000000000000000000000000000000000000006c47304402204bb1197053d0d7799bf1b30cd503c44b58d6240cccbdc85b6fe76d087980208f02204beeed78200178ffc6c74237bb74b3f276bbb4098b5605d814304fe128bf1431012321039e8815e15952a7c3fada1905f8cf55419837133bd7756c0ef14fc8dfe50c0deaacffffffff0001000000000000000000000000000000000000000000000000000000000000000000006c47304402202306489afef52a6f62e90bf750bbcdf40c06f5c6b138286e6b6b86176bb9341802200dba98486ea68380f47ebb19a7df173b99e6bc9c681d6ccf3bde31465d1f16b3012321039e8815e15952a7c3fada1905f8cf55419837133bd7756c0ef14fc8dfe50c0deaacffffffff010000000000000000015100000000");
+
+ let mut decoder = Transaction::decoder();
+ let mut slice = tx_bytes.as_slice();
+ decoder.push_bytes(&mut slice).unwrap();
+ let err = decoder.end().expect_err("transaction with duplicate inputs should be rejected");
+
+ let expected_outpoint = OutPoint {
+ txid: Txid::from_byte_array([
+ 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ ]),
+ vout: 0,
+ };
+ assert_eq!(err, TransactionDecoderError(TransactionDecoderErrorInner::DuplicateInput(expected_outpoint)));
+ }
}
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.