Decent - ZdravkoHr's results

Lines of code

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DcntEth.sol#L20-L22

Vulnerability details

Impact

DcntEth.setRouter is used to change the router address. The router is a privileged role that can mint and burn DcntEth tokens. setRouter can be called by anyone, meaning that a malicious user can create his own router which behaves the same as the original one, but with 1 change - he will add a function that allows him to mint/burn wherever they want. This can be unnoticed for a long time. The user will then have control over the whole protcol.

Proof of Concept

    function testAnyoneCanChangeRouter() public {
        vm.prank(address(0xb0b));
        dcntEth.setRouter(address(weth));
        assertEq(dcntEth.router(), address(weth));
    }

Tools Used

Foundry

Recommended Mitigation Steps

Add the onlyOwner modifier to DcntEth.setRouter()

-    function setRouter(address _router) public {
+    function setRouter(address _router) public onlyOwner {
        router = _router;
    }

Assessed type

Access Control

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

Lines of code

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

Vulnerability details

Impact

Any refunds from LayerZero will be sent to DecentBridgeAdapter, not to the initiator of the action. The adapter does not have any way to send this funds to the correct recipient, so they will be left in the contract, available for further use by other users.

Proof of Concept

1. User X wants to use DecentBridgeAdapter to execute a cross-chain action.
2. He calls UTB.bridgeAndExecute().
3. UTB.bridgeAndExecute calls UTB.callBridge(), which makes an internal call to DecentBridgeAdapter.bridge() and sends native tokens for gas.

            IBridgeAdapter(bridgeAdapters[instructions.bridgeId]).bridge{
                value: bridgeFee + (native ? amt2Bridge : 0)
            }(...);

4. DecentBridgeAdapter.bridge() calls DecentEthRouter.bridgeWithPayload() and forwards the native value.

        router.bridgeWithPayload{value: msg.value}(
            lzIdLookup[dstChainId],
            destinationBridgeAdapter[dstChainId],
            swapParams.amountIn,
            false,
            dstGas,
            bridgePayload
        );

5. DecentEthRouter.bridgeWithPayload calls the internal function _bridgeWithPayload().

6. It then constructs call parameters for layer zero and sets the refund address to msg.sender. If we look at step 4, it's clear that msg.sender is DecentBridgeAdapter. The funds will be sent to it and not to the initiator of the action, resulting in loss for the sender and profit for the next user that uses the protocol.

        ICommonOFT.LzCallParams memory callParams = ICommonOFT.LzCallParams({
            refundAddress: payable(msg.sender),
            zroPaymentAddress: address(0x0),
            adapterParams: adapterParams
        });

Tools Used

Manual Review

Recommended Mitigation Steps

Instead of msg.sender, use a parameter specified by the initiator of the action.

Assessed type

Other