Biconomy - Smart Contract Wallet contest - supernova's results

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L247-L269

Vulnerability details

Impact

Smart Account is the main account that handles the execution of the wallet transactions signed by the owner. It then refunds the tx executor/relayer for the gas cost.

There are 2 modes of refunding it :-

• Pay in Native Value i.e ETH
• Pay in ERC20 tokens .

The owner signs the relevant parameters that determine the above mode.

The relevant parameters that the owner signs are below:-

    /// @dev Returns the bytes that are hashed to be signed by owner.
    /// @param _tx Wallet transaction 
    /// @param refundInfo Required information for gas refunds
    /// @param _nonce Transaction nonce.
    /// @return Transaction hash bytes.
    function encodeTransactionData(
        Transaction memory _tx,
        FeeRefund memory refundInfo,
        uint256 _nonce
    ) public view returns (bytes memory) {
        bytes32 safeTxHash =
            keccak256(
                abi.encode(
                    ACCOUNT_TX_TYPEHASH,
                    _tx.to,
                    _tx.value,
                    keccak256(_tx.data),
                    _tx.operation,
                    _tx.targetTxGas,
                    refundInfo.baseGas,
                    refundInfo.gasPrice,
                    refundInfo.gasToken,
                    refundInfo.refundReceiver,
                    _nonce
                )
            );
        return abi.encodePacked(bytes1(0x19), bytes1(0x01), domainSeparator(), safeTxHash);
    }

Now look at the handlePayment function below:-

 function handlePayment(
        uint256 gasUsed,
        uint256 baseGas,
        uint256 gasPrice,
        uint256 tokenGasPriceFactor,
        address gasToken,
        address payable refundReceiver
    ) private nonReentrant returns (uint256 payment) {
        // uint256 startGas = gasleft();
        // solhint-disable-next-line avoid-tx-origin
        address payable receiver = refundReceiver == address(0) ? payable(tx.origin) : refundReceiver;
        if (gasToken == address(0)) {
            // For ETH we will only adjust the gas price to not be higher than the actual used gas price
            payment = (gasUsed + baseGas) * (gasPrice < tx.gasprice ? gasPrice : tx.gasprice);
            (bool success,) = receiver.call{value: payment}("");
            require(success, "BSA011");
        } else {
            payment = (gasUsed + baseGas) * (gasPrice) / (tokenGasPriceFactor);
            require(transferToken(gasToken, receiver, payment), "BSA012");
        }
        // uint256 requiredGas = startGas - gasleft();
        //console.log("hp %s", requiredGas);
    }

If the signer selects gasToken as some ERC20 token ,

The wallet transfers the ERC20 tokens to the receiver . This is the calculation below for it .

 payment = (gasUsed + baseGas) * (gasPrice) / (tokenGasPriceFactor);

Here baseGas, gasPrice is signed by the owner already . But the problem is tokenGasPriceFactor is not signed a.k.a omitted .

The following will be the consequences of this act -:

• The entire execution can be DOS

Attacker can frontrun the transaction and set tokenGasPriceFactor as 0. Now , the execution will revert due to Division or modulo by 0 error .

• Drain ERC20 tokens from the wallet

Attacker will frontrun the transaction and set tokenGasPriceFactor as 1 . Payment now will be = gasUsed + baseGas) * (gasPrice) This will lead to more tokens being sent to the receiver than should be .

Proof of Concept

Example :

These are the parameters set by the owner

• "baseGas": 29162,
• "gasPrice": "2025",
• "tokenGasPriceFactor": 1000000,
• "gasToken": "0xdA5289fCAAF71d52a80A254da614a192b693e977"

Here gasToken is USDC .

As tokenGasPriceFactor is not signed , attacker can frontrun it and change its value -:

• tokenGasPriceFactor = 0

payment  = 29162 * 2025 /  0 ;

It will revert due to division by 0 error .

• tokenGasPriceFactor = 1

payment = 25000 * 2025 / 1 ;

Payment = 50625000 a.k.a 50.625 USDC

Originally payment should have been = 0.000050 USDC

Hence wallet is overpaying the receiver by 1000000 times .

This attack is possible every time ERC20 token is used to refund the executor. Hence , I view it as a high severity bug .

Tools Used

Manual

Recommended Mitigation Steps

Sign tokenGasPriceFactor also .