fee: harden fee calldata parsing

bin/app/src/app/schema/wallet/send.rs

@@ -104,7 +104,7 @@ pub async fn make(
             let mut fees: u64 = 0;
             for call in tx.calls.iter() {
                 if call.data.is_money_fee() {
-                    if let Ok(fee) = darkfi_serial::deserialize(&call.data.data[1..9]) {
+                    if let Ok(fee) = call.data.money_fee_value() {
                         fees = fees.saturating_add(fee);
                     }
                 }

bin/darkfid/src/registry/model.rs

@@ -41,7 +41,8 @@ use darkfi::{
     Error, Result,
 };
 use darkfi_money_contract::{
-    client::pow_reward_v1::PoWRewardCallBuilder, MoneyFunction, MONEY_CONTRACT_ZKAS_MINT_NS_V1,
+    client::pow_reward_v1::PoWRewardCallBuilder, model::MoneyPoWRewardParamsV1, MoneyFunction,
+    MONEY_CONTRACT_ZKAS_MINT_NS_V1,
 };
 use darkfi_sdk::{
     crypto::{
@@ -49,6 +50,7 @@ use darkfi_sdk::{
         pasta_prelude::PrimeField,
         FuncId, MerkleTree, MONEY_CONTRACT_ID,
     },
+    fee::accumulate_fee,
     pasta::pallas,
     ContractCall,
 };
@@ -195,8 +197,22 @@ impl BlockTemplate {
     /// Note: always check if block contains transactions before
     /// calling this function.
     pub async fn reward(&self) -> Result<u64> {
-        Ok(deserialize_async::<u64>(&self.block.txs.last().unwrap().calls[0].data.data[1..9])
-            .await?)
+        let Some(producer_tx) = self.block.txs.last() else {
+            return Err(Error::BlockContainsNoTransactions(
+                self.block.header.template_hash().as_string(),
+            ))
+        };
+
+        let Some(call) = producer_tx.calls.first() else {
+            return Err(Error::ParseFailed("producer transaction contains no calls"))
+        };
+
+        if !call.data.is_money_pow_reward() {
+            return Err(Error::ParseFailed("producer transaction is not Money::PoWRewardV1"))
+        }
+
+        let params: MoneyPoWRewardParamsV1 = deserialize_async(&call.data.data[1..]).await?;
+        Ok(params.input.value)
     }
 
     /// Return block fees.
@@ -211,7 +227,7 @@ impl BlockTemplate {
                     continue
                 }
 
-                fees += deserialize_async::<u64>(&call.data.data[1..9]).await?;
+                fees = accumulate_fee(fees, call.data.money_fee_value()?)?;
                 continue 'outer
             }
         }
@@ -228,7 +244,7 @@ impl BlockTemplate {
         }
 
         let fees = self.fees().await?;
-        let reward = self.reward().await? - fees;
+        let reward = self.reward().await?.checked_sub(fees).ok_or(Error::SubtractionUnderflow)?;
 
         Ok((reward, fees))
     }

bin/drk/src/common.rs

@@ -27,7 +27,7 @@ use darkfi_sdk::{
     },
     pasta::pallas,
 };
-use darkfi_serial::{deserialize, serialize};
+use darkfi_serial::serialize;
 use prettytable::{format, row, Table};
 
 use crate::money::BALANCE_BASE10_DECIMALS;
@@ -232,7 +232,7 @@ pub fn pretty_tx(tx: &Transaction) -> String {
 
     for (i, call) in tx.calls.iter().enumerate() {
         if call.data.is_money_fee() {
-            if let Ok(fee) = deserialize(&call.data.data[1..9]) {
+            if let Ok(fee) = call.data.money_fee_value() {
                 fees.push(format!("{} DRK", encode_base10(fee, BALANCE_BASE10_DECIMALS)));
                 fees_total = fees_total.checked_add(fee).unwrap_or_else(|| {
                     fees_overflow = true;

bin/drk/src/money.rs

@@ -123,6 +123,24 @@ pub const MONEY_ALIASES_COL_TOKEN_ID: &str = "token_id";
 
 pub const BALANCE_BASE10_DECIMALS: usize = 8;
 
+const MONEY_FEE_PREFIX_LEN: usize = 9;
+
+fn parse_money_function(data: &[u8]) -> Result<MoneyFunction> {
+    let Some(func) = data.first() else {
+        return Err(Error::ParseFailed("money call data is empty"))
+    };
+
+    Ok(MoneyFunction::try_from(*func)?)
+}
+
+async fn parse_money_fee_params(data: &[u8]) -> Result<MoneyFeeParamsV1> {
+    if data.len() < MONEY_FEE_PREFIX_LEN {
+        return Err(Error::ParseFailed("money fee call data is too short"))
+    }
+
+    Ok(deserialize_async(&data[MONEY_FEE_PREFIX_LEN..]).await?)
+}
+
 impl Drk {
     /// Initialize wallet with tables for the Money contract.
     pub async fn initialize_money(&self, output: &mut Vec<String>) -> WalletDbResult<()> {
@@ -770,10 +788,10 @@ impl Drk {
 
         let call = &calls[*call_idx];
         let data = &call.data.data;
-        match MoneyFunction::try_from(data[0])? {
+        match parse_money_function(data)? {
             MoneyFunction::FeeV1 => {
                 scan_cache_log!(scan_cache, "[parse_money_call] Found Money::FeeV1 call");
-                let params: MoneyFeeParamsV1 = deserialize_async(&data[9..]).await?;
+                let params = parse_money_fee_params(data).await?;
                 nullifiers.push(params.input.nullifier);
                 if !params.output.tx_local {
                     coins.push((params.output.coin, params.output.note, false));
@@ -1080,9 +1098,9 @@ impl Drk {
         let mut nullifiers: Vec<Nullifier> = vec![];
 
         let data = &call.data.data;
-        match MoneyFunction::try_from(data[0])? {
+        match parse_money_function(data)? {
             MoneyFunction::FeeV1 => {
-                let params: MoneyFeeParamsV1 = deserialize_async(&data[9..]).await?;
+                let params = parse_money_fee_params(data).await?;
                 nullifiers.push(params.input.nullifier);
             }
             MoneyFunction::TransferV1 => {
@@ -1402,7 +1420,7 @@ impl Drk {
                 continue
             }
 
-            match MoneyFunction::try_from(call.data.data[0])? {
+            match parse_money_function(&call.data.data)? {
                 MoneyFunction::FeeV1 => {
                     return Err(Error::Custom("Fee call already exists".to_string()))
                 }

bin/explorer/src/rpc.rs

@@ -27,9 +27,7 @@ use darkfi::{
     tx::Transaction,
     util::{encoding::base64, parse::encode_base10},
 };
-use darkfi_money_contract::MoneyFunction;
-use darkfi_sdk::crypto::contract_id::MONEY_CONTRACT_ID;
-use darkfi_serial::{deserialize_async, serialize_async};
+use darkfi_serial::serialize_async;
 use monero::{consensus::encode::Encodable, VarInt};
 use tiny_keccak::{Hasher, Keccak};
 use tinyjson::JsonValue;
@@ -82,15 +80,15 @@ impl TransactionInfo {
         let mut fee = 0;
         let mut calls = Vec::with_capacity(tx.calls.len());
         for call in &tx.calls {
-            let func = call.data.data[0];
+            let func = call.data.data.first().copied();
 
-            if call.data.contract_id == *MONEY_CONTRACT_ID && func == MoneyFunction::FeeV1 as u8 {
-                fee = deserialize_async(&call.data.data[1..9]).await.unwrap();
+            if let Ok(parsed_fee) = call.data.money_fee_value() {
+                fee = parsed_fee;
             }
 
             calls.push(ContractCallInfo::new(
                 call.data.contract_id.to_string(),
-                format!("0x{:02x}", func),
+                func.map(|func| format!("0x{func:02x}")).unwrap_or_else(|| "empty".to_string()),
                 call.data.data.len() as u64,
             ));
         }
@@ -132,15 +130,15 @@ impl ExplTxInfo {
         let mut fee = 0;
         let mut calls = Vec::with_capacity(tx.calls.len());
         for call in &tx.calls {
-            let func = call.data.data[0];
+            let func = call.data.data.first().copied();
 
-            if call.data.contract_id == *MONEY_CONTRACT_ID && func == MoneyFunction::FeeV1 as u8 {
-                fee = deserialize_async(&call.data.data[1..9]).await.unwrap();
+            if let Ok(parsed_fee) = call.data.money_fee_value() {
+                fee = parsed_fee;
             }
 
             calls.push(ContractCallInfo::new(
                 call.data.contract_id.to_string(),
-                format!("0x{:02x}", func),
+                func.map(|func| format!("0x{func:02x}")).unwrap_or_else(|| "empty".to_string()),
                 call.data.data.len() as u64,
             ));
         }

script/research/tx-replayer/src/main.rs

@@ -301,8 +301,8 @@ async fn verify_transaction_wasm(
     let total_gas_used = gas_data.total_gas_used();
 
     if verify_fee {
-        // Deserialize the fee call to find the paid fee
-        let fee: u64 = match deserialize_async(&tx.calls[fee_call_idx].data.data[1..9]).await {
+        // Extract the paid fee from the fee call.
+        let fee = match tx.calls[fee_call_idx].data.money_fee_value() {
             Ok(v) => v,
             Err(_) => return Err(TxVerifyFailed::InvalidFee.into()),
         };
@@ -476,8 +476,8 @@ async fn verify_transaction_zkps(
     let total_gas_used = gas_data.total_gas_used();
 
     if verify_fee {
-        // Deserialize the fee call to find the paid fee
-        let fee: u64 = match deserialize_async(&tx.calls[fee_call_idx].data.data[1..9]).await {
+        // Extract the paid fee from the fee call.
+        let fee = match tx.calls[fee_call_idx].data.money_fee_value() {
             Ok(v) => v,
             Err(_) => return Err(TxVerifyFailed::InvalidFee.into()),
         };
@@ -630,8 +630,8 @@ async fn verify_transaction_signatures(
     let total_gas_used = gas_data.total_gas_used();
 
     if verify_fee {
-        // Deserialize the fee call to find the paid fee
-        let fee: u64 = match deserialize_async(&tx.calls[fee_call_idx].data.data[1..9]).await {
+        // Extract the paid fee from the fee call.
+        let fee = match tx.calls[fee_call_idx].data.money_fee_value() {
             Ok(v) => v,
             Err(_) => return Err(TxVerifyFailed::InvalidFee.into()),
         };

src/contract/money/src/entrypoint.rs

@@ -19,7 +19,7 @@
 use darkfi_sdk::{
     crypto::{pasta_prelude::Field, smt::EMPTY_NODES_FP, ContractId, MerkleNode, MerkleTree},
     dark_tree::DarkLeaf,
-    error::ContractResult,
+    error::{ContractError, ContractResult},
     msg,
     pasta::pallas,
     wasm, ContractCall,
@@ -40,6 +40,12 @@ use crate::{
     MONEY_CONTRACT_NULLIFIER_ROOTS_TREE, MONEY_CONTRACT_TOKEN_FREEZE_TREE,
 };
 
+fn parse_money_function(data: &[u8]) -> Result<MoneyFunction, ContractError> {
+    let Some(func) = data.first() else { return Err(ContractError::InvalidFunction) };
+
+    MoneyFunction::try_from(*func)
+}
+
 /// `Money::Fee` functions
 mod fee_v1;
 use fee_v1::{
@@ -230,7 +236,7 @@ fn get_metadata(cid: ContractId, ix: &[u8]) -> ContractResult {
     let call_idx = wasm::util::get_call_index()? as usize;
     let calls: Vec<DarkLeaf<ContractCall>> = deserialize(ix)?;
     let self_ = &calls[call_idx].data;
-    let func = MoneyFunction::try_from(self_.data[0])?;
+    let func = parse_money_function(&self_.data)?;
 
     let metadata = match func {
         MoneyFunction::FeeV1 => {
@@ -263,7 +269,7 @@ fn process_instruction(cid: ContractId, ix: &[u8]) -> ContractResult {
     let call_idx = wasm::util::get_call_index()? as usize;
     let calls: Vec<DarkLeaf<ContractCall>> = deserialize(ix)?;
     let self_ = &calls[call_idx].data;
-    let func = MoneyFunction::try_from(self_.data[0])?;
+    let func = parse_money_function(&self_.data)?;
 
     let update_data = match func {
         MoneyFunction::FeeV1 => {
@@ -302,7 +308,7 @@ fn process_instruction(cid: ContractId, ix: &[u8]) -> ContractResult {
 /// is the update data retrieved from `process_instruction()`, prefixed with the
 /// contract function.
 fn process_update(cid: ContractId, update_data: &[u8]) -> ContractResult {
-    match MoneyFunction::try_from(update_data[0])? {
+    match parse_money_function(update_data)? {
         MoneyFunction::FeeV1 => {
             let update: MoneyFeeUpdateV1 = deserialize(&update_data[1..])?;
             Ok(money_fee_process_update_v1(cid, update)?)

src/contract/money/src/entrypoint/fee_v1.rs

@@ -50,6 +50,20 @@ use crate::{
     MONEY_CONTRACT_NULLIFIER_ROOTS_TREE, MONEY_CONTRACT_ZKAS_FEE_NS_V1,
 };
 
+const MONEY_FEE_PREFIX_LEN: usize = 9;
+
+fn parse_fee_call_data(data: &[u8]) -> Result<(u64, MoneyFeeParamsV1), ContractError> {
+    if data.len() < MONEY_FEE_PREFIX_LEN {
+        msg!("[FeeV1] Error: Fee call data is too short");
+        return Err(MoneyError::InvalidFeeCall.into())
+    }
+
+    let fee = deserialize(&data[1..MONEY_FEE_PREFIX_LEN])?;
+    let params = deserialize(&data[MONEY_FEE_PREFIX_LEN..])?;
+
+    Ok((fee, params))
+}
+
 /// `get_metadata` function for `Money::FeeV1`
 pub(crate) fn money_fee_get_metadata_v1(
     _cid: ContractId,
@@ -57,9 +71,7 @@ pub(crate) fn money_fee_get_metadata_v1(
     calls: Vec<DarkLeaf<ContractCall>>,
 ) -> Result<Vec<u8>, ContractError> {
     let self_ = &calls[call_idx].data;
-    // The first 8 bytes here is the u64 fee, so we get the params from that offset.
-    // (Plus 1, which is the function identifier byte)
-    let params: MoneyFeeParamsV1 = deserialize(&self_.data[9..])?;
+    let (_, params) = parse_fee_call_data(&self_.data)?;
 
     // Public inputs for the ZK proofs we have to verify
     let mut zk_public_inputs: Vec<(String, Vec<pallas::Base>)> = vec![];
@@ -103,8 +115,7 @@ pub(crate) fn money_fee_process_instruction_v1(
     calls: Vec<DarkLeaf<ContractCall>>,
 ) -> Result<Vec<u8>, ContractError> {
     let self_ = &calls[call_idx];
-    let fee: u64 = deserialize(&self_.data.data[1..9])?;
-    let params: MoneyFeeParamsV1 = deserialize(&self_.data.data[9..])?;
+    let (fee, params) = parse_fee_call_data(&self_.data.data)?;
 
     // We should have _some_ fee paid...
     if fee == 0 {

src/contract/money/src/error.rs

@@ -114,6 +114,9 @@ pub enum MoneyError {
 
     #[error("Invalid local output")]
     InvalidLocalOutput,
+
+    #[error("Invalid fee call")]
+    InvalidFeeCall,
 }
 
 impl From<MoneyError> for ContractError {
@@ -150,6 +153,7 @@ impl From<MoneyError> for ContractError {
             MoneyError::ChildrenIndexesLengthMismatch => Self::Custom(29),
             MoneyError::BurnMissingInputs => Self::Custom(30),
             MoneyError::InvalidLocalOutput => Self::Custom(31),
+            MoneyError::InvalidFeeCall => Self::Custom(32),
         }
     }
 }

src/validator/verification.rs

@@ -889,13 +889,13 @@ pub async fn verify_transaction(
     }
 
     if verify_fee {
-        // Deserialize the fee call to find the paid fee
-        let fee: u64 = match deserialize_async(&tx.calls[fee_call_idx].data.data[1..9]).await {
+        // Extract the paid fee from the fee call.
+        let fee = match tx.calls[fee_call_idx].data.money_fee_value() {
             Ok(v) => v,
             Err(e) => {
                 error!(
                     target: "validator::verification::verify_transaction",
-                    "[VALIDATOR] Failed deserializing tx {tx_hash} fee call: {e}"
+                    "[VALIDATOR] Failed parsing tx {tx_hash} fee call: {e}"
                 );
                 return Err(TxVerifyFailed::InvalidFee.into())
             }