[Feature]: Insecure tx-sender principal authentication and Post-Conditions reliance

[evonide]

(High) Insecure tx-sender principal authentication and Post-Conditions reliance

1. Description

The sBTC Clarity contracts rely on tx-sender for principal authentication in two ways.

• To verify the current principal is a signer.

  • Multiple such as sbtc-bootstrap-signers:rotate-keys-wrapper

• To spend tokens from tx-sender or to ensure one is allowed to do so from a given sender address.

  • sbtc-token:transfer uses

     (asserts! (or (is-eq tx-sender sender) (is-eq contract-caller sender)) ERR_NOT_OWNER)
    

    to transfer sBTC from a given sender to a given recipient.
  • sbtc-withdrawal:initiate-withdrawal-request uses

     (try! (contract-call? .sbtc-token protocol-lock amount tx-sender))
    

    to lock sBTC from the tx-sender in order to issue BTC at a later point.

The usage of tx-sender seems to be recommended by SIP-010 as Stacks token standards don't have a notion of approvals. However, checks against tx-sender can be implicitly bypassed once there's a call into a malicious contract. To mitigate this, sBTC relies on Stacks Post-Conditions. These checks, specified outside of contract code, run in deny mode by default, meaning no asset transfers can occur except the ones explicitly allowed. However, there are notable limitations:

• You can only track outgoing asset transfers from principals but not incoming asset transfers.
• State changes cannot be tracked.
• If a contract uses as-contract, it changes tx-sender to its own address. Post-conditions are set once, before the transaction is signed, and cannot be accessed or modified during contract execution. Accordingly, if a contract (e.g., Foo) calls as-contract and subsequently calls sbtc-token:transfer, an attacker could specify a post-condition such as [Foo spends ≥ 0 sBTC] and then drain all sBTC from Foo.

Recommendations for sBTC

• Avoid using tx-sender for any auth purposes altogether as the phishing risk is insufficiently mitigated by Post-Conditions. Instead, consider implementing an approval-based mechanism such as ERC-1363 approveAndCall or ERC-2612 style permits to do so in a single transaction.

Long-term considerations for Stacks

• Introduce the concept of post-conditions to as-contract calls.
• Post-conditions are hard to use so it's very foreseeable that providers will end up using too broad margins in practice to avoid breaking user experience.
• Post-conditions UX in wallets can be improved. For example, specifying [Principal spends ≥ 0.01 STX] can lead to full STX drainage without displaying any warnings.

3. Related Issues and Pull Requests (optional):

• "Add possibility to secure function calls with post-condition" (Add possibility to secure functin calls with post-condition stacks-core#2921)
• "Authorization using or and tx-sender" (Authorization using or and tx-sender Trust-Machines/BNS-V2#70)

[jcnelson]

If a contract uses as-contract, it changes tx-sender to its own address. Post-conditions are set once, before the transaction is signed, and cannot be accessed or modified during contract execution. Accordingly, if a contract (e.g., Foo) calls as-contract and subsequently calls sbtc-token:transfer, an attacker could specify a post-condition such as [Foo spends ≥ 0 sBTC] and then drain all sBTC from Foo.

This is equivalent to tricking the user to sign a malicious transaction, which is not something the blockchain can address. How can the blockchain know that the user was tricked?

The wallet would display the list of post-conditions to the user, and the user would check that they are sound. For example, a user would expect that the post-conditions only authorize the user's address, and nothing else, and would not sign a transaction which stipulated that Foo can spend tokens.

We believe that requiring the user to inspect post-conditions is a reasonable defense against malicious websites submitting malicious transactions to the user to sign, because post-conditions are something that non-technical users can readily understand with the help of their wallet (as opposed to, e.g., the code body of the contract and the arguments to the public function). For example, the wallet can represent the user's account address as "You" or "Your account" in the post-condition list, so the user would immediately identify the presence of an unrecognized principal's post-condition, and refuse to sign. As another example, the wallet can flag principal spends >= X tokens as dangerous, since they do not constrain how much of an asset can be moved.

[evonide]

Thanks Jude,

This is equivalent to tricking the user to sign a malicious transaction, which is not something the blockchain can address. How can the blockchain know that the user was tricked?

The described example could have been clearer. The described risk here is not that the user sings a malicious transaction but instead that an attacker can call into a vulnerable contract Foo which in turn could call into an attacker controlled contract Bar using as-contract for example because Foo accepts a user supplied contract to call to. In this instance, post-conditions won't be able to protect Foo's assets as an attacker can just specify the post-condition [Foo spends ≥ 0 sBTC] to drain all sBTC from Foo. Hopefully that makes more sense? An example of this attack against the Zest protocol is described in https://exvul.com/a-new-attack-on-bitcoin-defi/ resulting in the theft of 183,548 STX at the time.

This specific vector is known as per stacks-network/stacks-core#2921 but hasn't been addressed.

As another example, the wallet can flag principal spends >= X tokens as dangerous, since they do not constrain how much of an asset can be moved.

Agreed, however as noted the available wallets don't seem to show any warnings for unconstrained post-conditions or at least didn't do so at the time of issue creation. Please see:

Post-conditions UX in wallets can be improved. For example, specifying [Principal spends ≥ 0.01 STX] can lead to full STX drainage without displaying any warnings.

[obycode]

I agree that the post-conditions should be sufficient to keep users safe. There has been discussion before about the need for a mechanism for a contract to protect itself, such as as-contract post-conditions or mechanisms to validate balances after the call. It seems like everyone is in agreement that something in this area would be helpful, but it has not been able to reach the top of the prioritization stack yet or had anyone take the lead in implementation. In addition to the issue you mentioned, see also stacks-network/stacks-core#3094.