#38740 [BC-High] The missing check in Deposits::DepositScriptInputs::parse() permits losing funds by

Submitted on Jan 11th 2025 at 16:53:15 UTC by @jovemjeune for Attackathon | Stacks

• Report ID: #38740

• Report Type: Blockchain/DLT

• Report severity: High

• Target: https://github.com/stacks-network/sbtc/tree/immunefi_attackaton_0.9/sbtc

• Impacts:

  • A bug in the respective layer 0/1/2 network code that results in unintended smart contract behavior with no concrete funds at direct risk

Description

Brief/Intro

Provide a very short and concise (one paragraph) statement on what the problem is, and what the consequences would be if the bug were exploited in production/main-net.

On stacks blockchain funds can only be sent to the principals start with either SP or ST however it is actually possible creating a deposit transaction with an invalid principal for recipient and if a user triggers such operation it will use his funds.

Vulnerability Details

Offer a detailed explanation of the vulnerability itself. Do not leave out any relevant information. Code snippets should be supplied whenever helpful, as long as they don’t overcrowd the report with unnecessary details. This section should make it obvious that you understand exactly what you’re talking about, and more importantly, it should be clear by this point that the vulnerability does exist.

Deposits::DepositScriptInputs::parse() method is designed for checking whether the deposit script is valid or not and it specifically checks if the script format matches with the following <deposit-data> OP_DROP OP_PUSHBYTES_32 <x-only-public-key> OP_CHECKSIG. It also parses the stack address by the following lines in the url : https://github.com/stacks-network/sbtc/blob/immunefi_attackaton_0.9/sbtc/src/deposits.rs#L338-L339 however these lines does not check if the receipent address is valid or not which permits depositing funds to an in invalid address.

Impact Details

Provide a detailed breakdown of possible losses from an exploit, especially if there are funds at risk. This illustrates the severity of the vulnerability, but it also provides the best possible case for you to be paid the correct amount. Make sure the selected impact is within the program’s list of in-scope impacts and matches the impact you selected.

Funds can be sent to a malformed principal which is not valid according to clarity book which means if the users can ended up loosing their funds in case they attempt depositing their funds through a malformed principal even though such principal should normally rejected(since stacks does not accept a principal starts with S0 or anything other than those begin with SP or ST relative to the blockchain either mainnet or testnet ).

References

Add any relevant links to documentation or code

https://book.clarity-lang.org/ch02-01-primitive-types.html#:~:text=Principals%20follow%20a%20specific%20structure%20and%20always%20start%20with%20the%20characters%20SP%20for%20the%20Stacks%20mainnet%20and%20ST%20for%20the%20testnet%20and%20mocknet1.

https://github.com/stacks-network/sbtc/blob/immunefi_attackaton_0.9/sbtc/src/deposits.rs#L285-L348

Link to Proof of Concept

https://gist.github.com/Taneristique/dfc96d55f6d0bcd2d3648631ba45600a

Proof of Concept

Proof of Concept

The issue can be regenerated by copying the function below on https://github.com/stacks-network/sbtc/blob/immunefi_attackaton_0.9/sbtc/src/deposits.rs#L575 or changing https://github.com/stacks-network/sbtc/blob/immunefi_attackaton_0.9/sbtc/src/deposits.rs as the gist file shows.

 fn parse_invalid_address_data(){
        // Import missing dependencies 
        use bitcoin::transaction::Version;
        use bitcoin::Amount;
        use bitcoin::absolute::LockTime;
        use bitcoin::Transaction;
        use bitcoin::TxOut;
        use crate::deposits;
    
        // Create a secret key for the depositor and set the maximum fee 
        let secret_key = SecretKey::new(&mut OsRng);
        let max_fee: u64 = 15000;
        // Initialize a deposit with an invalid recipient principal it starts neither with 'SP' or 'ST'. 
        let lock_time=0;
        let deposit = DepositScriptInputs {
            signers_public_key: secret_key.x_only_public_key(SECP256K1).0,
            recipient: clarity::vm::types::PrincipalData::Standard(StandardPrincipalData{0:0,1:[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]}),
            max_fee,
        };
        let reclaim = ReclaimScriptInputs::try_new(lock_time, ScriptBuf::new()).unwrap();
    
        // Set a deposit amount
        let amount= 10000;  
    
        // Create the deposit script  
        let deposit_script = deposit.deposit_script();
        //create a reclaim script 
        let reclaim_script = reclaim.reclaim_script();
    
    
      
        // Check if deposit and reclaim scripts are valid by using parse methdod
        let result = DepositScriptInputs::parse(&deposit_script);
        let result2 = ReclaimScriptInputs::parse(&reclaim_script);
    
        // Demonstrate on console that deposit is accepted as valid even though stacks has no valid principal format such S0000000000000000000002AA028H
        println!("Incorrect  Principal : {}",StandardPrincipalData::to_address(&StandardPrincipalData{0:0,1:[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]}));
        println!("Correct Principal : {}",StandardPrincipalData::to_address(&StandardPrincipalData::from(StacksAddress::burn_address(false))));
        println!("Even thought receipent principal is incorrect , deposit script error status : {:?} and reclaim script error status : {:?}", &result.err(), &result2.err()); 
    
        // Prepare  the transaction 
        let tx = Transaction {
            version: Version::TWO,
            lock_time: LockTime::ZERO,
            input: Vec::new(),
            output: vec![TxOut {
                value: Amount::from_sat(amount),
                script_pubkey: deposits::to_script_pubkey(deposit_script, reclaim_script),
            }],
        };
        impl std::fmt::Debug for TxSetup{
            fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
                f.debug_struct("TxSetup").field("tx", &self.tx).field("deposit", &self.deposit).field("reclaim", &self.reclaim).finish()
            }
        }
        // Print the transaction data on console 
        println!("{:#?}",TxSetup { tx, deposit, reclaim })
    
    }

After running the test it will give following log:

warning: `sbtc` (lib test) generated 3 warnings (run `cargo fix --lib -p sbtc --tests` to apply 3 suggestions)
    Finished `test` profile [unoptimized] target(s) in 1.39s
     Running unittests src/lib.rs (target/debug/deps/sbtc-d0130838370bf5e1)

running 1 test
test deposits::tests::parse_invalid_address_data ... ok

successes:

---- deposits::tests::parse_invalid_address_data stdout ----
Incorrect  Principal : S0000000000000000000002AA028H
Correct Principal : ST000000000000000000002AMW42H
Even thought receipent principal is incorrect , deposit script error status : None and reclaim script error status : None
TxSetup {
    tx: Transaction {
        version: Version(
            2,
        ),
        lock_time: 0 blocks,
        input: [],
        output: [
            TxOut {
                value: 10000 SAT,
                script_pubkey: Script(OP_PUSHNUM_1 OP_PUSHBYTES_32 203b7a12cf8fc789fbe1ef4c76947188d41d9f7407251244a8a251a6f2a5c803),
            },
        ],
    },
    deposit: DepositScriptInputs {
        signers_public_key: XOnlyPublicKey(
            0b257e31fa4058df1bf872275b7e1433777deeb82802c5288cc0ea9584b24eed72a36f006371895849a2d7908b67878dca317ca51be95802e2d47a98ec856971,
        ),
        recipient: Standard(
            StandardPrincipalData(S0000000000000000000002AA028H),
        ),
        max_fee: 15000,
    },
    reclaim: ReclaimScriptInputs {
        lock_time: Blocks(
            Height(
                0,
            ),
        ),
        script: Script(),
    },
}


successes:
    deposits::tests::parse_invalid_address_data

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 66 filtered out; finished in 0.00s

which clearly shows that the reciepent address is S0000000000000000000002AA028H and invalid.