Decent - rouhsamad's results

Lines of code

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

Vulnerability details

Impact

setRouter function does not have any access controls, allowing anyone to change router to an arbitrary address and mint unlimited tokens.

Proof of Concept

mint function is onlyRouter, which means router (or exploiter address) is able to mint unlimited tokens: https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DcntEth.sol#L24-L26

Tools Used

• Manual review

Recommended Mitigation Steps

setRouter should be declared as onlyOwner:

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

Assessed type

Access Control

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#L250-L255

Vulnerability details

Proof of Concept

When receiving a cross-chain message, the onOFTReceived function of DecentRouter is invoked. Now, assuming that this message contains a payload for calling the destination (_to) , we can reasonably infer that _to is most likely a smart contract (though it might also be an EOA, but in most cases, it is a smart contract). The issue arises when there is an insufficient amount of WETH in the DecentEthRouter. In this scenario, Decent tokens are sent to _to, which is a smart contract. Consequently, there is a high risk that these Decent tokens might be locked indefinitely in the smart contract, leading to potential loss of funds.

Impact

loss of funds - The likelihood of encountering this issue is very high, given that users utilize the UTB contract for making cross-chain swaps. in this case if the DecentEthRouter at destination lacks a sufficient amount of WETH, Decent tokens are then sent to the DecentBridgeAdapter in the destination chain, which, crucially, lacks any functions for withdrawing these tokens. This situation poses a significant risk as there is no mechanism in place for retrieving or managing the Decent tokens sent under these circumstances.

Tools Used

• Manual review

Recommended Mitigation Steps

In this scenario, I would suggest implementing a check to verify if the message contains a payload. If it does, there is no reason for calling _to when there isn't enough WETH in the contract. It would be more secure to redirect the funds to the refund address. Conversely, if the message doesn't contain a payload and _to is an externally owned account (EOA), it is deemed safe to send Decent tokens to either _to or the refund address.

Assessed type

Invalid Validation

Lines of code

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L285-L291 https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L275-L276 https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L313-L319

Vulnerability details

Impact

High - imho the impact of this issue is should be classified as high due to its substantial likelihood, simplicity in execution (requiring only a single function call), and the potential for substantial financial losses. Users face a considerable risk, as demonstrated by scenarios such as swapping 1 Ethereum for 1 Matic, resulting in significant loss of funds.

Proof of Concept

In some chains, such as Polygon/AVAX, Ethereum is not recognized as their gas token. This distinction is addressed in the DecentEthRouter by configuring the gasCurrencyIsEth variable as either true or false. However, some sections of both DecentEthRouter and DecentBridgeExecutor (as mentioned below) are mistaking Ethereum as the native chain token without checking gasCurrencyIsEth.

DecentEthRouter::redeemEth: When a user wants to redeem their Decent tokens (on Polygon/Avax) using this function, the user's Decent tokens are transferred to the contract, WETH is withdrawn (burned), and native chain tokens (e.g., Matic in the Polygon example) are sent to the user. This implies that if, for example, Alice decides to redeem her 10 dcntEth tokens, she will receive 10 Matic tokens, which are worth only 7 USD, resulting in the loss of all her 10 WETH (worth 22,000 usd)

DecentBridgeExecutor::_executeETH: Also in this function, WETH is withdrawn and Matic/Avax are received which might be useless when calling the target. this function results in loss of funds for user (from) and stuck funds in DecentBridgeExecutor which can be withdrawn by a malicious actor.

NOTE: there might not be enough MATIC/AVAX in router in order to be sent to user, however, a malicious actor is able to fund contract with enough MATIC/AVAX.

Code PoC: while the issue is clear, i have decided to put a code PoC (Copy from next line to before Tools Used and paste in a new file, no additional settings needed):

//SPDX-License-Identifier: MIT

pragma solidity ^0.8.13;

import {Test, console, console2} from "forge-std/Test.sol"; import {IERC20} from "@openzeppelin/contracts/interfaces/IERC20.sol"; import {ERC20Mock} from "@openzeppelin/contracts/mocks/ERC20Mock.sol";

interface IWETH { function withdraw(uint amt) external payable;

function deposit() external payable;

function balanceOf(address user) external view returns (uint);

}

contract MockRouter is Test { IWETH polygon_weth = IWETH(0x7ceB23fD6bC0adD59E62ac25578270cFf1b9f619); IWETH wrappedMatic = IWETH(0x0d500B1d8E8eF31E21C99d1Db9A6444d3ADf1270);

ERC20Mock dcntToken;
address alice;

modifier onlyIfWeHaveEnoughReserves(uint256 amount) {
    require(
        polygon_weth.balanceOf(address(this)) > amount,
        "not enough reserves"
    );
    _;
}

function setUp() public {
    vm.createSelectFork("https://polygon.meowrpc.com");
    alice = makeAddr("Alice");
    dcntToken = new ERC20Mock();
    dcntToken.mint(alice, 10 ether);
    //transfer WETH to router
    vm.startPrank(0x62ac55b745F9B08F1a81DCbbE630277095Cf4Be1);
    IERC20(address(polygon_weth)).transfer(address(this), 11 ether);
    vm.stopPrank();
}

function redeemEth(
    uint256 amount
) public onlyIfWeHaveEnoughReserves(amount) {
    dcntToken.transferFrom(msg.sender, address(this), amount);
    polygon_weth.withdraw(amount);
    payable(msg.sender).transfer(amount);
}

function testWithdrawWETH() public {
    //alice deciding to withdraw his 10 Dcnt tokens for ether
    assertEq(dcntToken.balanceOf(alice), 10 ether);
    assertEq(alice.balance, 0); //no matics
    assertEq(polygon_weth.balanceOf(alice), 0); //no WETH
    assertEq(wrappedMatic.balanceOf(alice), 0); //alice doesn't have any MATIC

    vm.startPrank(alice);
    dcntToken.approve(address(this), 10 ether);
    this.redeemEth(10 ether);
    wrappedMatic.deposit{value: alice.balance}();

    assertEq(dcntToken.balanceOf(alice), 0);//tokens sent to the contract
    assertEq(alice.balance, 0); //no matics
    assertEq(polygon_weth.balanceOf(alice), 0); //no WETH
    assertEq(wrappedMatic.balanceOf(alice), 10 ether); //alice has 10 matics instead of WETH 
    vm.stopPrank();
}

receive() external payable {}

}

Tools Used

Manual review Forge testing

Mitigation steps

While I don't have a clear solution in mind yet, I would suggest first checking gasCurrencyIsEth to determine whether the chain supports ETH as the gas token or not. If it does not (false), then proceed to swap WETH for the native chain token (e.g., WETH => MATIC) and send MATIC tokens to the user. Alternatively, consider not allowing certain functions to be executed if the chain does not support ETH as the native gas token.

Assessed type

Invalid Validation