c4-bot-10 commented 9 months ago

Lines of code

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentBridgeExecutor.sol#L35-L38 https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentBridgeExecutor.sol#L62-L64

Vulnerability details

Impact

Smart contracts that interact with the protocol may not receive their refund in case a transaction fails on the destination chain. This is because refunds are sent to the from address of the source chain. Smart contacts are not guaranteed to have the same address on different chains. This means that any funds sent to from address that is not controlled by the protocol interacting with Decent will probably be lost.

Proof of Concept

There are two (Gnosis) Safes controlled by a protocol on two different chains. Since they are smart contracts, their addresses can be different. The protocol wants to use Decent for cross-chain execution. For some reason, their transaction fails on the destination chain.

DecentBridgeExecutor will send the funds to the from address

    function _executeEth(
        address from,
        address target,
        uint256 amount,
        bytes memory callPayload
    ) private {
        weth.withdraw(amount);
        (bool success, ) = target.call{value: amount}(callPayload);
        if (!success) {
            // here
            payable(from).transfer(amount);
        }
    }

This from will be different from the address of the smart contract on the currentChain. Funds will be send to the wrong address and will be lost.

Tools Used

Recommended Mitigation Steps

Add a new parameter address destinationRefund that can be specified on the source chain. Then use it instead of from.

 if (!success) {
-            payable(from).transfer(amount);
+            payable(destinationRefund).transfer(amount);
 }

Assessed type

Other

c4-pre-sort commented 9 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 9 months ago

raymondfam marked the issue as duplicate of #27

c4-judge commented 8 months ago

alex-ppg marked the issue as partial-75