Skip to content

JDS needs tighter validation against DeclareMiningJob.coinbase_tx_prefix #645

Description

@plebhash

JDS's DeclaredCustomJob::get_coinbase_tx is a bit too naive:

fn get_coinbase_tx(&self) -> Result<Transaction, ()> {
let declared_coinbase_tx_prefix: Vec<u8> =
self.declare_mining_job.coinbase_tx_prefix.to_owned_bytes();
let declared_coinbase_tx_suffix: Vec<u8> =
self.declare_mining_job.coinbase_tx_suffix.to_owned_bytes();
// Parse scriptSig size from coinbase prefix
// Coinbase structure: version(4) + marker+flag(2) + input_count(1) + outpoint(32) +
// index(4) = 43 bytes Then comes scriptSig length (VarInt) followed by scriptSig
// data
const COINBASE_PREFIX_LEN: usize = 43;
let script_sig_size: usize = {
let mut cursor = &declared_coinbase_tx_prefix[COINBASE_PREFIX_LEN..];
match bitcoin::VarInt::consensus_decode(&mut cursor) {
Ok(varint) => varint.0 as usize,
Err(e) => {
tracing::error!(
"Failed to decode scriptSig size from coinbase prefix: {}",
e
);
return Err(());
}
}
};
// Calculate the size of scriptSig bytes already in the prefix.
let varint_size = bitcoin::VarInt(script_sig_size as u64).size();
let script_sig_offset = COINBASE_PREFIX_LEN + varint_size;
let script_sig_bytes_in_prefix = declared_coinbase_tx_prefix.len() - script_sig_offset;
// The full extranonce fills the remaining space in scriptSig
let full_extranonce_size: usize = script_sig_size - script_sig_bytes_in_prefix;
// Concatenate prefix + full extranonce (zeros) + suffix to form the complete transaction
// bytes
let mut declared_coinbase_tx = declared_coinbase_tx_prefix;
declared_coinbase_tx.extend_from_slice(&vec![0; full_extranonce_size]);
declared_coinbase_tx.extend_from_slice(&declared_coinbase_tx_suffix);

if a DeclareMiningJob comes with coinbase_tx_prefix that don't satisfy some naive assumptions, that can lead to panics

more specifically:

  • if coinbase_tx_prefix is smaller than the expected 43 bytes, &declared_coinbase_tx_prefix[COINBASE_PREFIX_LEN..] panics with index-out-of-bounds
  • adjacent arithmetic (prefix.len() - script_sig_offset, script_sig_size - script_sig_bytes_in_prefix) can also underflow depending on the contents of DeclareMiningJob
  • if scriptSig length is declared as a sufficiently large value (which would actually break Bitcoin consensus), it could lead to unbounded memory allocation

in summary, JDS should defensively check against all these edge cases before allocating memory, and simply reject the DeclareMiningJob in case of invalid values

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    Status
    Todo 📝
    Status
    Todo 📝

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions