code-423n4 / 2022-07-axelar-findings

0 stars 0 forks source link

Malicious DepositBase may stole dust fund from ReceiverImplementation #206

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L16-L40

Vulnerability details

Impact

Malicious DepositBase may stole dust fund from ReceiverImplementation

Proof of Concept

    // @dev This function is used for delegate by DepositReceiver deployed above
    // Context: msg.sender == AxelarDepositService, this == DepositReceiver
    function receiveAndSendToken(
        address payable refundAddress,
        string calldata destinationChain,
        string calldata destinationAddress,
        string calldata symbol
    ) external {
        // Always refunding native otherwise it's sent on DepositReceiver self destruction
        if (address(this).balance > 0) refundAddress.transfer(address(this).balance);

        address tokenAddress = IAxelarGateway(gateway).tokenAddresses(symbol);
        // Checking with AxelarDepositService if need to refund a token
        address refund = DepositBase(msg.sender).refundToken();
        if (refund != address(0)) {
            _safeTransfer(refund, refundAddress, IERC20(refund).balanceOf(address(this)));
            return;
        }

        uint256 amount = IERC20(tokenAddress).balanceOf(address(this));

        if (amount == 0) revert NothingDeposited();

        // Sending the token trough the gateway
        IERC20(tokenAddress).approve(gateway, amount);
        IAxelarGateway(gateway).sendToken(destinationChain, destinationAddress, symbol, amount);
    }

Attacker just craft a malicious DepositBase with refundToken has a value of token address that he want to steal. And call ReceiverImplementation.receiveAndSendToken from that contract to get both dust native token from if (address(this).balance > 0) refundAddress.transfer(address(this).balance); and ERC20 token from _safeTransfer(refund, refundAddress, IERC20(refund).balanceOf(address(this)));

Tools Used

Manual review

Recommended Mitigation Steps

Whitelist the valid AxelarDepositService

re1ro commented 2 years ago

Funds gets deposited to the specific deposit addresses that are generated from the original AxelarDepositService. The address of AxelarDepositService is used for the deposit address computation. If some other contract will implement AxelarDepositService and will try to deploy a DepositReceiver. It will get deployed to a completely different address and wont be able to access the funds

GalloDaSballo commented 2 years ago

I have to agree with the Sponsor, the DepositReceiver is deposited via create2 while we can predict the address of multiple DepositReceivers by mining the salt, we would have to break secp256k1 to be able to create a new contract that can deploy at that same address

Closing as invalid because of that reason, in lack of POC