TOB-FUEL-4: claim_refund does not refund the original asset

[xgreenx]

Description

Funds in the FuelERC20Gateway can be drained due to returning an incorrect asset when claiming a refund.
Refunds are registered when there is a mismatch between the L1 and L2 assets.

Refunds are accounted for in the register_refund function and stored in the storage variable refund_amounts.

Figure 4.2: The register_refund function in bridge-fungible-token/bridge-fungible-token/src/bridge_fungible_token.sw

// Storage-dependant private functions
#[storage(write)]
fn register_refund(from: b256, asset: b256, amount: b256) {
    storage.refund_amounts.get(from).insert(asset, amount);
    log(RefundRegisteredEvent {
        from,
        asset,
        amount,
    });
}

For a user to withdraw registered refunds the claim_refund function should be called. This function however incorrectly sets as the token to be withdrawn the BRIDGED_TOKEN instead of the asset variable.

Figure 4.3: The claim_refund function in bridge-fungible-token/bridge-fungible-token/src/bridge_fungible_token.sw

130    fn claim_refund(originator: b256, asset: b256) {
131       let stored_amount =
storage.refund_amounts.get(originator).get(asset).read();
132       require(stored_amount != ZERO_B256,
BridgeFungibleTokenError::NoRefundAvailable);
133
134       // reset the refund amount to 0
135       storage.refund_amounts.get(originator).insert(asset, ZERO_B256);
136
137       // send a message to unlock this amount on the base layer gateway contract
138       send_message(BRIDGED_TOKEN_GATEWAY, encode_data(originator, stored_amount,
BRIDGED_TOKEN), 0);
139    }

This allows an attacker to deposit any fake asset and to withdraw the true asset when claiming a refund.

Figure 4.4: The finalizeWithdrawal function in fuel-v2-contracts/contracts/messaging/gateway/FuelERC20Gateway.sol

    function finalizeWithdrawal(
        address to,
        address tokenId,
        uint256 amount
    ) external payable whenNotPaused onlyFromPortal {
        require(amount > 0, "Cannot withdraw zero");
        bytes32 fuelTokenId = messageSender();

        //reduce deposit balance and transfer tokens (math will underflow if amount is larger than allowed)
        _deposits[tokenId][fuelTokenId] = _deposits[tokenId][fuelTokenId] - amount;
        IERC20Upgradeable(tokenId).safeTransfer(to, amount);

        //emit event for successful token withdraw
        emit Withdrawal(bytes32(uint256(uint160(to))), tokenId, fuelTokenId, amount);
    }

This is possible because there is no deposit accounting per user in the FuelERC20Gateway which allows for withdrawals to different addresses other than the depositors.

Exploit Scenario

Alice creates a fake token and bridges 10K FAKE while specifying the Fuel ETH token id on the Fuel chain. Because the bridged asset is not appropriate, Alice is able to claim a refund. Due to the error, she is able to refund 10K ETH on Ethereum.

Recommendations

Short term, update the encoded data to include the deposited asset instead of the bridge asset. Alternatively, the gateway could be re-designed to not require specifying the Fuel token id on Ethereum which could eliminate the need for refunds.
Long term, pay extra attention to the intersection between blockchains. Make sure these are well-documented and tested, since they are typically harder to test and more prone to errors.

[xgreenx]

Fixed by #4, duplicate of #5