Skip to content

Commit dd1fa91

Browse files
committed
duplicate input detection
1 parent 70a7c38 commit dd1fa91

4 files changed

Lines changed: 51 additions & 3 deletions

File tree

crates/floresta-chain/src/pruned_utreexo/consensus.rs

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,24 @@ impl Consensus {
296296
return Err(tx_err!(txid, NullPrevOut))?;
297297
}
298298

299-
// Check script sizes (current tx scriptsig and TODO witness if present)
299+
// Witness size is intentionally not checked here — witness-specific
300+
// limits are enforced during script execution, not in context-free checks.
301+
// This matches Bitcoin Core's CheckTransaction() which explicitly skips
302+
// witness in context-free checks because witness data has not been
303+
// checked for malleability at this point.
304+
// See: https://github.com/bitcoin/bitcoin/blob/master/src/consensus/tx_check.cpp
300305
Self::validate_script_size(&input.script_sig, txid)?;
301-
// TODO check also witness script size
306+
}
307+
308+
// Check for duplicate inputs (CVE-2018-17144).
309+
// UpdateCoins does not detect duplicates — a duplicate prevout causes either
310+
// a crash or an inflation bug depending on the coins database implementation.
311+
// Bitcoin Core catches this explicitly in CheckTransaction() for the same reason.
312+
let mut seen = HashSet::new();
313+
for input in &transaction.input {
314+
if !seen.insert(&input.previous_output) {
315+
return Err(tx_err!(txid, DuplicateInput))?;
316+
}
302317
}
303318

304319
let out_value = transaction
@@ -983,6 +998,24 @@ mod tests {
983998
}
984999
}
9851000

1001+
#[test]
1002+
fn test_duplicate_inputs_rejected() {
1003+
let outpoint = dummy_outpoint();
1004+
1005+
// Same prevout used in both inputs
1006+
let tx = build_tx(
1007+
vec![txin!(outpoint), txin!(outpoint)],
1008+
vec![txout!(0, ScriptBuf::new())],
1009+
);
1010+
1011+
match Consensus::check_transaction_context_free(&tx) {
1012+
Err(BlockchainError::TransactionError(tx_err)) => {
1013+
assert_eq!(tx_err.error, BlockValidationErrors::DuplicateInput);
1014+
}
1015+
other => panic!("Expected DuplicateInput, got: {other:?}"),
1016+
}
1017+
}
1018+
9861019
#[test]
9871020
fn test_input_value_above_max_money() {
9881021
let outpoint = dummy_outpoint();

crates/floresta-chain/src/pruned_utreexo/error.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ pub enum BlockValidationErrors {
8585
CoinbaseNotMatured,
8686
UnspendableUTXO,
8787
BIP94TimeWarp,
88+
DuplicateInput,
8889
}
8990

9091
// Helpful macro for generating a TransactionError
@@ -175,6 +176,9 @@ impl Display for BlockValidationErrors {
175176
BlockValidationErrors::BIP94TimeWarp => {
176177
write!(f, "BIP94 time warp detected")
177178
}
179+
BlockValidationErrors::DuplicateInput => {
180+
write!(f, "This transaction has duplicate inputs")
181+
}
178182
}
179183
}
180184
}

crates/floresta-mempool/src/mempool.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,11 @@ mod tests {
421421
};
422422

423423
let inputs = rng.gen_range(1..10);
424+
425+
// Track outpoints already used in this transaction to avoid
426+
// duplicate inputs within a single tx, which is never valid.
427+
// Conflicts are only meaningful across separate transactions.
428+
let mut used_in_this_tx = HashSet::new();
424429
for _ in 0..inputs {
425430
if outputs.is_empty() {
426431
break;
@@ -432,6 +437,11 @@ mod tests {
432437
true => *outputs.get(index).unwrap(),
433438
};
434439

440+
// Skip if this outpoint is already an input in this tx
441+
if !used_in_this_tx.insert(previous_output) {
442+
continue;
443+
}
444+
435445
let input = bitcoin::TxIn {
436446
previous_output,
437447
script_sig: bitcoin::Script::new().into(),
@@ -505,7 +515,7 @@ mod tests {
505515
for tx in transactions {
506516
match mempool.accept_to_mempool(tx) {
507517
Ok(_) => {}
508-
Err(AcceptToMempoolError::DuplicatedInputs) => {
518+
Err(AcceptToMempoolError::ConflictingTransaction) => {
509519
did_conflict = true;
510520
}
511521

crates/floresta-wire/src/p2p_wire/node/blocks.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,7 @@ where
357357
BlockValidationErrors::InvalidCoinbase(_)
358358
| BlockValidationErrors::ScriptValidationError(_)
359359
| BlockValidationErrors::NullPrevOut
360+
| BlockValidationErrors::DuplicateInput
360361
| BlockValidationErrors::EmptyInputs
361362
| BlockValidationErrors::EmptyOutputs
362363
| BlockValidationErrors::ScriptError

0 commit comments

Comments
 (0)