Address to refund the tokens being bridged is incorrectly assigned and will cause users to lose their funds in case the bridge execution fails in the destination chain.

[c4-bot-9]

Lines of code

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L101 https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L105

Vulnerability details

Impact

Users will lost all their bridged funds in case that the bridge execution fails in the destination chain.

Proof of Concept

When a user is bridging tokens to perform an execution in the destination chain, the users initiates their transaction by:

1. Calling the UTB::bridgeAndExecute() function, from there it will do the swap pre-bridge (in the src chain) & will modify the swapInstructions for the swap post-bridge (in the dest chain), then it will invoke the UTB::callBridge() function
2. In the UTB::callBridge() function some approvals are granted to the DecentBridgeAdapter contract, and then the execution is forwarded to the DecentBridgeAdapter::bridge() function
3. In the DecentBridgeAdapter::bridge() function, some data is encoded and decoded to prepare the calldata for the bridge, and then the [DecentEthRouter::bridgeWithPayload() function]() is called.
4. When the execution reaches the DecentEthRouter::bridgeWithPayload() function, notice how the msg.sender in the DecentEthRouter contract will be the DecentBridgeAdapter contract, (from step 3 to step 4, the caller is the DecentBridgeAdapter contract). When the payload that is sent for the execution in the destination chain is defined in the DecentEthRouter::_getCallParams() function, which is called by the DecentEthRouter::_bridgeWithPayload() function, the second parameter that is encoded as part of the payload variable it is set to be the msg.sender, keep in mind that the msg.sender is the address of the DecentBridgeAdapter, not the user's address or an address controlled by him.
5. From here onwards, the execution is sent to LayerZero, which will send the payload variable to the [DecentEthRouter::onOFTReceived() function]() in the destination chain.

6. When the DecentEthRouter::onOFTReceived() function is called, the received _payload is decoded and the encoded values are saved in individual variables, the one we are interested is the second variable, which is the msg.sender that was encoded in the DecentEthRouter::_getCallParams() function, then, if the crosschain message happens to be of an execute type, the execution will be forwarded to the DecentBridgeExecutor::execute() function
7. In the DecentBridgeExecutor::execute() function, either the DecentBridgeExecutor::_executeWeth() function or the DecentBridgeExecutor::_executeEth() function are called, and the received from parameter (which was just decoded from the received payload in the onOFTReceived()) is forwarded to those functions.

8. In any of those functions, the execution will call the target contract (which is the DecentBridgeAdapter::receiveFromBridge() of the current chain).
9. If the call to the target contract reverts, all the bridged funds are refunded to the from parameter, which is exactly the msg.sender that was encoded in the DecentEthRouter::_getCallParams() function. (See step 4)
   • Refunding to the msg.sender who called the DecentEthRouter::_getCallParams() function in the source chain will cause that the users lost all his bridged funds, and instead, the DecentBridgeAdapter contract address of the source chain will be the one that received the refund of the bridged funds.

Tools Used

Manual Audit

Recommended Mitigation Steps

Instead of hardcoding the refundAddress to be the msg.sender, use the refundee address that was already provided by the user. This address was passed in the calldata sent to the UTB::bridgeAndExecute() function, specifically, the refundAddress specified by the user is encoded in the instructions.refund variable.

• Make sure to decode the encoded calldata that gets to the DecentEthRouter::_getCallParams() function. Read the value of the instructions.refund variable that was sent at the beginning of the execution, and use that address to set the second variable of the payload variable that is passed as a parameter for the execution in the destination chain.

Assessed type

en/de-code

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #27

[c4-judge]

alex-ppg marked the issue as satisfactory