code-423n4 / 2024-01-decent-findings

3 stars 3 forks source link

Anyone can update the address of the Router in the DcntEth contract to any address they would like to set. #721

Open c4-bot-8 opened 10 months ago

c4-bot-8 commented 10 months ago

Lines of code

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

Vulnerability details

Impact

By allowing anybody to set the address of the Router contract to any address they want to set it allows malicious users to get access to the mint and burn functions of the DcntEth contract.

Proof of Concept

The DcntEth::setRouter() function has not an access control to restrict who can call this function. This allows anybody to set the address of the router contract to any address they'd like to set it.

DcntEth.sol

//@audit-issue => No access control to restrict who can set the address of the router contract
function setRouter(address _router) public {
router = _router;
}

The functions DcntEth::mint() function & DcntEth::burn() function can be called only by the router contract.

DcntEth.sol

//@audit-info => Only the router can call the mint()
function mint(address _to, uint256 _amount) public onlyRouter {
    _mint(_to, _amount);
}

//@audit-info => Only the router can call the burn()
function burn(address _from, uint256 _amount) public onlyRouter {
    _burn(_from, _amount);
}

A malicious user can set the address of the router contract to an account of their own and:
1. Gain access to mint unlimited amounts of DcntEth token, which could be used to disrupt the crosschain accounting mechanism, or to steal the deposited weth in the DecentEthRouter contract.
2. Burn all the DcntEth tokens that were issued to the DecentEthRouter contract when liquidity providers deposited their WETH or ETH into it.
3. Cause a DoS to the add and remove liquidity functions of the DecentEthRouter contract. All of these functions end up calling the [`DcntEth::mint() function`](https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DcntEth.sol#L24-L26) or the [`DcntEth::burn() function`](https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DcntEth.sol#L28-L30), if the router address is set to be different than the address of the DecentEthRouter, all the calls made from the DecentEthRouter to the DcnEth contract will revert.

> DecentEthRouter.sol
```solidity

    /// @inheritdoc IDecentEthRouter
    function addLiquidityEth()
        public
        payable
        onlyEthChain
        userDepositing(msg.value)
    {
        weth.deposit{value: msg.value}();

        //@audit-issue => If router in the dcnteth contract is not set to the address of the DecentEthRouter, this call will revert
        dcntEth.mint(address(this), msg.value);
    }

    /// @inheritdoc IDecentEthRouter
    function removeLiquidityEth(
        uint256 amount
    ) public onlyEthChain userIsWithdrawing(amount) {

      //@audit-issue => If router in the dcnteth contract is not set to the address of the DecentEthRouter, this call will revert
        dcntEth.burn(address(this), amount);
        weth.withdraw(amount);
        payable(msg.sender).transfer(amount);
    }

    /// @inheritdoc IDecentEthRouter
    function addLiquidityWeth(
        uint256 amount
    ) public payable userDepositing(amount) {
        weth.transferFrom(msg.sender, address(this), amount);

        //@audit-issue => If router in the dcnteth contract is not set to the address of the DecentEthRouter, this call will revert
        dcntEth.mint(address(this), amount);
    }

    /// @inheritdoc IDecentEthRouter
    function removeLiquidityWeth(
        uint256 amount
    ) public userIsWithdrawing(amount) {

      //@audit-issue => If router in the dcnteth contract is not set to the address of the DecentEthRouter, this call will revert
        dcntEth.burn(address(this), amount);
        weth.transfer(msg.sender, amount);
    }

Tools Used

Manual Audit

Recommended Mitigation Steps

Make sure to add an Acess Control mechanism to limit who can set the address of the Router in the DcnEth contract.

Assessed type

Access Control

c4-pre-sort commented 10 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #14

c4-judge commented 10 months ago

alex-ppg marked the issue as satisfactory

alex-ppg commented 10 months ago

This and all relevant submissions correctly specify that the lack of access control in the DcntEth::setRouter function can be exploited maliciously and effectively compromise the entire TVL of the Decent ETH token.

A high-risk severity is appropriate, and this submission was selected as the best due to detailing all possible impacts:

c4-judge commented 10 months ago

alex-ppg marked the issue as selected for report

c4-sponsor commented 9 months ago

wkantaros (sponsor) confirmed