Decent - bart1e's results

Lines of code

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L266-L269 https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L271-L277 https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L289 https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L298 https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L317 https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L334

Vulnerability details

DecentETHRouter::onOFTReceived will be invoked when users successfully bridge their tokens to another blockchain. Depending on the value of msgType extracted from the _payload argument, it will either just transfer funds or use them to execute some operation.

However, when there is not enough WETH, it will simply transfer DcntEth to the recipient, no matter whether msgType is MT_ETH_TRANSFER or MT_ETH_TRANSFER_WITH_PAYLOAD.

If attacker manages to drain WETH from DcntEthRouter, then he will essentially block every MT_ETH_TRANSFER_WITH_PAYLOAD operation. Moreover, he will also block calls to all functions that transfer out WETH:

• redeemWeth
• redeemEth
• removeLiquidityEth
• removeLiquidityWeth

The exact attack scenario is described in the PoC section. Note that exact same mechanism can be used to drain DcntEthRouter from DcntEth and block DcntEthRouter usage.

Impact

Anyone is able to drain the entire WETH balance from DcntEthRouter, effectively blocking users from performing MT_ETH_TRANSFER_WITH_PAYLOAD operations and blocking WETH providers from removing liquidity. Moreover, users won't be able to redeem their DcntEth for WETH or ETH.

Attacker can also block DcntEthRouter on a source chain by draining its DcntEth balance.

Proof of Concept

One way of draining WETH from DcntEthRouter on a blockchain X is described below:

1. Attacker bridges some funds, say 1 WETH to X from another blockchain Y using the Decent protocol.
2. Router will either weth.transfer(_to, _amount) or weth.withdraw(_amount), so its WETH balance will decrease.
3. Attacker will use another bridge, not related to the Decent protocol and will bridge his 1 WETH back to Y.
4. He will repeat the procedure described in steps 1-3 as many times as necessary, until WETH.balanceOf(DcntEthRouter) == 0.

In exactly the same way, an attacker can drain DcntEthRouter on Y from DcntEth as it will call DcntEth with itself as DcntEth provider.

Tools Used

VS Code

Recommended Mitigation Steps

Probably the best solution to this problem would involve cyclically bridging WETH from DcntEthRouters that have a lot of it to the routers having small amount of it. The same mechanism can be used for DcntEth.

The attack will still be possible, but won't be so damaging for the protocol.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/UTB.sol#L311-L319 https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/UTB.sol#L235-L248 https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/UTB.sol#L141-L160

Vulnerability details

Users can swap their tokens and execute custom actions using UTB::swapAndExecute function. Although, in theory they could pass any arguments for that function, they will get immediately validated in retrieveAndCollectFees when a call to feeCollector.collectFees is made. feeCollector.collectFees will perform the validation of both fees and instructions arguments making sure that:

• the fees are specified correctly
• Uniswap swap path is correct
• call targets for UTBExecutor along with their calldata are correct

However, there is another function, called receiveFromBridge, that is permissionless and allows users to call _swapAndExecute without any validation of arguments passed. It is intended to be only called when funds are bridged, but current code doesn't enforce it and anyone can call it.

It means that any user will be able to pass any arguments to _swapAndExecute he wants:

• it can be a fake path for Uniswap swap (so that, for example tokenOut is different from the token really swapped - it would allow him to drain any token from UniSwapper by specifying target token as tokenOut and a pair of fake tokens in the path)
• UTBExecutor will be forced to call any contract with any calldata. Attacker can, for example encode <ANY_ERC20>.approve(attacker, type(uint).max) or even a direct transfer of any token, so he can drain any token from UTBExecutor he wants.

The additional impact is that since retrieveAndCollectFees will not be executed, any user can always avoid paying any fees.

Impact

Any user can drain the entire UniSwapper and UTBExecutor balance.

In case of UTBExecutor it is expected for it to contain some tokens, because it refunds tokens that weren't spend to the refund address and the amount of tokens refunded is calculated as a difference between the current and starting balance. If the contract was expected to always contain 0 tokens, initBalance would always be 0 instead of IERC20(token).balanceOf(address(this)).

Moreover, any user can always avoid paying a fee for his swap-and-execute action.

Proof of Concept

UTB::receiveFromBridge is permissionless and it doesn't have retrieveAndCollectFees modifier. Moreover, it will call _swapAndExecute.

_swapAndExecute will perform any requested swap and will call UTBExecutor with any calldata specified:

        (address tokenOut, uint256 amountOut) = performSwap(swapInstructions);
        if (tokenOut == address(0)) {
            executor.execute{value: amountOut}(
                target,
                paymentOperator,
                payload,
                tokenOut,
                amountOut,
                refund
            );
        } else {
            IERC20(tokenOut).approve(address(executor), amountOut);
            executor.execute(
                target,
                paymentOperator,
                payload,
                tokenOut,
                amountOut,
                refund
            );
        }

Finally, UTBExecutor::execute will invoke any contract with any calldata.

Tools Used

VS Code

Recommended Mitigation Steps

Add access control to receiveFromBridge, so that it can only be invoked by certain contracts when funds are received from the bridge.

Assessed type

Access Control