Skip to content

Commit

Permalink
Fixed no zeroing malleable fields for Create transaction (#767)
Browse files Browse the repository at this point in the history 
* Fixed no zeroing malleable fields for `Create` transaction

* Updated CHANGELOG.md
  • Loading branch information
@xgreenx
xgreenx authored Jun 15, 2024
1 parent e041eff commit 3816c54
Show file tree
Hide file tree
Showing 4 changed files with 227 additions and 97 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Changed

#### Breaking
- [#765](https://github.com/FuelLabs/fuel-vm/pull/765): corrected the gas units for WDOP and WQOP

- [#767](https://github.com/FuelLabs/fuel-vm/pull/767): Fixed no zeroing malleable fields for `Create` transaction.
- [#765](https://github.com/FuelLabs/fuel-vm/pull/765): Corrected the gas units for WDOP and WQOP.

## [Version 0.53.0]

Expand Down
7 changes: 0 additions & 7 deletions fuel-vm/src/interpreter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -528,11 +528,6 @@ pub trait ExecutableTransaction:
}) if *input_index as usize == input)
})
}

/// Prepares the transaction for execution.
fn prepare_init_execute(&mut self) {
self.prepare_sign()
}
}

impl ExecutableTransaction for Create {
Expand All @@ -547,8 +542,6 @@ impl ExecutableTransaction for Create {
fn transaction_type() -> Word {
TransactionRepr::Create as Word
}

fn prepare_init_execute(&mut self) {}
}

impl ExecutableTransaction for Script {
Expand Down
2 changes: 1 addition & 1 deletion fuel-vm/src/interpreter/initialization.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ where
runtime_balances: RuntimeBalances,
gas_limit: Word,
) -> Result<(), RuntimeError<S::DataError>> {
tx.prepare_init_execute();
tx.prepare_sign();
self.tx = tx;

self.initial_balances = initial_balances.clone();
Expand Down
311 changes: 223 additions & 88 deletions fuel-vm/src/tests/validation.rs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
use crate::{
checked_transaction::{
CheckPredicateParams,
EstimatePredicates,
},
prelude::{
MemoryInstance,
*,
},
};
use core::str::FromStr;
use fuel_asm::{
op,
Expand All @@ -6,14 +16,17 @@ use fuel_asm::{
use fuel_tx::{
field::{
Inputs,
Outputs,
ReceiptsRoot,
StorageSlots,
},
input::coin::CoinSigned,
input::coin::CoinPredicate,
ConsensusParameters,
Input,
TransactionBuilder,
UtxoId,
};
use fuel_types::BlockHeight;
use fuel_vm::prelude::*;
use rand::{
rngs::StdRng,
Rng,
Expand Down Expand Up @@ -53,115 +66,237 @@ fn transaction_can_be_executed_after_maturity() {
assert!(result.is_ok());
}

/// Malleable fields should not affect validity of the block
/// Malleable fields should not affect validity of the create transaction
#[test]
fn malleable_fields_do_not_affect_validity() {
let rng = &mut StdRng::seed_from_u64(8586);

fn malleable_fields_do_not_affect_validity_of_create() {
let params = ConsensusParameters::default();

let tx_start_ptr = params.tx_params().tx_offset();
let tx_size_ptr = tx_start_ptr - 8;
let witness_count_offset =
<Create as StorageSlots>::storage_slots_offset_static() - 8;

let tx = TransactionBuilder::script(
vec![
// Log tx id (hash)
op::movi(0x21, 32),
op::logd(0x00, 0x00, 0x00, 0x21),
// Load tx size
op::movi(0x21, tx_size_ptr as u32),
op::lw(0x21, 0x21, 0),
// Make heap space for tx bytes, chain_id and computed tx hash
op::addi(0x22, 0x21, 8 + 32),
op::aloc(0x22),
// Construct the chain_id and tx bytes for hashing
op::addi(0x22, RegId::HP, 32), // Chain id position
op::gtf_args(0x20, 0x00, GTFArgs::ScriptData),
op::mcpi(0x22, 0x20, 8), // Copy chain id
op::movi(0x20, tx_start_ptr as u32),
op::addi(0x23, 0x22, 8), // Tx bytes position
op::mcp(0x23, 0x20, 0x21), // Copy tx bytes
// Assert that there is exactly one witness
op::gtf_args(0x26, 0x00, GTFArgs::ScriptWitnessesCount),
op::eq(0x26, 0x26, RegId::ONE),
op::jnzf(0x26, 0x00, 1),
op::ret(0),
// Zero out the only witness (in the heap)
op::gtf_args(0x26, 0x00, GTFArgs::WitnessData),
op::gtf_args(0x27, 0x00, GTFArgs::WitnessDataLength),
op::sub(0x27, 0x26, 0x20), // Offset in relative to the tx bytes
op::add(0x27, 0x27, RegId::HP), // Redirect the pointer to heap
op::addi(0x26, 0x27, 32 + 8), // Offset of tx bytes in heap
op::gtf_args(0x27, 0x00, GTFArgs::WitnessDataLength),
op::mcl(0x26, 0x27),
// Zero out witness count
op::gtf_args(0x26, 0x00, GTFArgs::Script),
op::subi(0x26, 0x26, 8), // Offset to get the witness count address
op::sub(0x26, 0x26, 0x20), // Offset in relative to the tx bytes
op::add(0x26, 0x26, RegId::HP), // Redirect the pointer to heap
op::addi(0x26, 0x26, 32 + 8), // Offset of tx bytes in heap
op::sw(0x26, RegId::ZERO, 0), // Zero out the witness count
// Actually hash
op::subi(0x24, 0x21, 64 + 8 - 8), // Offset ptr
op::s256(RegId::HP, 0x22, 0x24), // Compute tx id hash
op::movi(0x25, 32), // Hash size
op::logd(0x00, 0x00, RegId::HP, 0x25), // Log computed txid
op::logd(0, 0, 0x22, 0x24),
// Done
op::ret(0x00),
]
.into_iter()
.collect(),
params.chain_id().to_be_bytes().to_vec(),
)
.add_unsigned_coin_input(
SecretKey::random(rng),
UtxoId::new([3; 32].into(), 0),
123456789,
AssetId::default(),
Default::default(),
)
.script_gas_limit(1_000_000)
.finalize();

let run_tx = |tx: Script| {
let original_id = tx.id(&params.chain_id());

let vm = Interpreter::<_, _, Script>::with_memory_storage();
let mut client = MemoryClient::from_txtor(vm.into());
let receipts =
client.transact(tx.into_checked(0u32.into(), &params).expect("valid tx"));

let start_id = receipts[0].data().unwrap();
let computed_id = receipts[1].data().unwrap();
let predicate_bytecode = [
// Load tx size
op::movi(0x21, tx_size_ptr as u32),
op::lw(0x21, 0x21, 0),
// Make heap space for tx bytes, chain_id and computed tx hash
op::addi(0x22, 0x21, 8 + 32),
op::aloc(0x22),
// Construct the chain_id and tx bytes for hashing
op::addi(0x22, RegId::HP, 32), // Chain id position
op::gtf_args(0x20, 0x00, GTFArgs::InputCoinPredicateData),
op::mcpi(0x22, 0x20, 8), // Copy chain id
op::movi(0x20, tx_start_ptr as u32),
op::addi(0x23, 0x22, 8), // Tx bytes position
op::mcp(0x23, 0x20, 0x21), // Copy tx bytes
// Assert that there is exactly one witness
op::gtf_args(0x26, 0x00, GTFArgs::ScriptWitnessesCount),
op::eq(0x26, 0x26, RegId::ONE),
op::jnzf(0x26, 0x00, 1),
op::ret(0),
// Zero out witness count
op::addi(0x26, 0x23, witness_count_offset as u16), // Offset to witness count
op::sw(0x26, RegId::ZERO, 0), // Zero out the witness count
// Actually hash. We use property that contract bytecode has zero size.
op::s256(RegId::HP, 0x22, 0x21), // Compute tx id hash
op::movi(0x25, 32), // Hash size
// Compare two hashes
op::meq(0x26, RegId::HP, 0x0, 0x25),
// Done
op::ret(0x26),
]
.into_iter()
.collect();
let predicate_data = params.chain_id().to_be_bytes().to_vec();
let predicate_owner = Input::predicate_owner(&predicate_bytecode);

assert_eq!(*original_id, start_id);
assert_eq!(*original_id, computed_id);
let mut tx = TransactionBuilder::create(vec![].into(), Salt::zeroed(), vec![])
.add_input(Input::coin_predicate(
UtxoId::new([4; 32].into(), 0),
predicate_owner,
123456789,
AssetId::default(),
Default::default(),
0,
predicate_bytecode,
predicate_data,
))
.add_output(Output::contract_created(
Contract::EMPTY_CONTRACT_ID,
Default::default(),
))
.add_output(Output::change(
Default::default(),
Default::default(),
Default::default(),
))
.finalize();
tx.estimate_predicates(&CheckPredicateParams::from(&params), MemoryInstance::new())
.expect("Should estimate predicate");

original_id
};

let original = run_tx(tx.clone());
let run_tx = |tx: Create| tx.into_checked(0u32.into(), &params).map(|_| ());
let result = run_tx(tx.clone());
assert_eq!(result, Ok(()));

// Check that modifying the input coin doesn't affect validity
{
let mut tx = tx.clone();

match tx.inputs_mut()[0] {
Input::CoinSigned(CoinSigned {
Input::CoinPredicate(CoinPredicate {
ref mut tx_pointer, ..
}) => *tx_pointer = TxPointer::from_str("123456780001").unwrap(),
_ => unreachable!(),
};

match tx.outputs_mut()[1] {
Output::Change { ref mut amount, .. } => {
*amount = 123456789;
}
_ => unreachable!(),
};
let result = run_tx(tx);
assert_eq!(result, original);
assert_eq!(result, Ok(()));
}
}

/// Malleable fields should not affect validity of the script
#[test]
fn malleable_fields_do_not_affect_validity_of_script() {
let params = ConsensusParameters::default();

let tx_start_ptr = params.tx_params().tx_offset();
let tx_size_ptr = tx_start_ptr - 8;

let predicate_bytecode = [
// Load tx size
op::movi(0x21, tx_size_ptr as u32),
op::lw(0x21, 0x21, 0),
// Make heap space for tx bytes, chain_id and computed tx hash
op::addi(0x22, 0x21, 8 + 32),
op::aloc(0x22),
// Construct the chain_id and tx bytes for hashing
op::addi(0x22, RegId::HP, 32), // Chain id position
op::gtf_args(0x20, 0x00, GTFArgs::InputCoinPredicateData),
op::mcpi(0x22, 0x20, 8), // Copy chain id
op::movi(0x20, tx_start_ptr as u32),
op::addi(0x23, 0x22, 8), // Tx bytes position
op::mcp(0x23, 0x20, 0x21), // Copy tx bytes
// Actually hash
op::addi(0x24, 0x21, 8), // Offset ptr
op::s256(RegId::HP, 0x22, 0x24), // Compute tx id hash
op::movi(0x25, 32), // Hash size
// Compare two hashes
op::meq(0x26, RegId::HP, 0x0, 0x25),
// Done
op::ret(0x26),
]
.into_iter()
.collect();
let predicate_data = params.chain_id().to_be_bytes().to_vec();
let predicate_owner = Input::predicate_owner(&predicate_bytecode);

// Check that modifying the receipts root doesn't affect validity
let mut tx = TransactionBuilder::script(vec![], vec![])
.add_input(Input::coin_predicate(
UtxoId::new([4; 32].into(), 0),
predicate_owner,
123456789,
AssetId::default(),
Default::default(),
0,
predicate_bytecode,
predicate_data,
))
.add_input(Input::contract(
Default::default(),
Default::default(),
Default::default(),
Default::default(),
Contract::EMPTY_CONTRACT_ID,
))
.add_output(Output::variable(
Default::default(),
Default::default(),
Default::default(),
))
.add_output(Output::change(
Default::default(),
Default::default(),
Default::default(),
))
.add_output(Output::contract(1, Default::default(), Default::default()))
.script_gas_limit(1_000_000)
.finalize();
tx.estimate_predicates(&CheckPredicateParams::from(&params), MemoryInstance::new())
.expect("Should estimate predicate");

let run_tx = |tx: Script| tx.into_checked(0u32.into(), &params).map(|_| ());
let result = run_tx(tx.clone());
assert_eq!(result, Ok(()));

// Check that modifying the input coin doesn't affect validity
{
let mut tx = tx;
let mut tx = tx.clone();

*tx.receipts_root_mut() = [1u8; 32].into();

match tx.inputs_mut()[0] {
Input::CoinPredicate(CoinPredicate {
ref mut tx_pointer, ..
}) => *tx_pointer = TxPointer::from_str("123456780001").unwrap(),
_ => unreachable!(),
};

match tx.inputs_mut()[1] {
Input::Contract(input::contract::Contract {
ref mut utxo_id,
ref mut balance_root,
ref mut state_root,
ref mut tx_pointer,
..
}) => {
*utxo_id = UtxoId::new([1; 32].into(), 0);
*balance_root = [2; 32].into();
*state_root = [3; 32].into();
*tx_pointer = TxPointer::from_str("123456780001").unwrap();
}
_ => unreachable!(),
};

match tx.outputs_mut()[0] {
Output::Variable {
ref mut to,
ref mut amount,
ref mut asset_id,
} => {
*to = [123; 32].into();
*amount = 123456789;
*asset_id = [213; 32].into();
}
_ => unreachable!(),
};

match tx.outputs_mut()[1] {
Output::Change { ref mut amount, .. } => {
*amount = 123456789;
}
_ => unreachable!(),
};

match tx.outputs_mut()[2] {
Output::Contract(output::contract::Contract {
ref mut balance_root,
ref mut state_root,
..
}) => {
*balance_root = [7; 32].into();
*state_root = [8; 32].into();
}
_ => unreachable!(),
};

let result = run_tx(tx);
assert_eq!(result, original);
assert_eq!(result, Ok(()));
}
}

0 comments on commit 3816c54

Please sign in to comment.