Risk of Unhandled Failures

[c4-bot-4]

Lines of code

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L285
https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L302

Vulnerability details

In the DecentEthRouter contract, there are several instances where external calls are made to other contracts, such as weth.transfer, weth.withdraw, weth.deposit, and dcntEth contract calls. These calls are potentially unsafe as they don't handle the possibility of these calls failing or reverting:

function redeemEth(uint256 amount) public onlyIfWeHaveEnoughReserves(amount) {
    // ...
    weth.withdraw(amount);
    payable(msg.sender).transfer(amount);
}

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

function removeLiquidityWeth(uint256 amount) public userIsWithdrawing(amount) {
    dcntEth.burn(address(this), amount);
    weth.transfer(msg.sender, amount);
}

Impact

If external calls to WETH or other contracts fail, the current implementation does not handle these failures, which can lead to transaction reversion and inconsistent contract states.

Mitigation

To address these risks, handle the possibility of failures in external calls:

• Implement checks for the return value or use try-catch blocks for external calls.

     function safeWethWithdraw(IWETH wethToken, uint256 amount) private {
         bool success = wethToken.withdraw(amount);
         require(success, "WETH withdrawal failed");
     }

     function safeEthTransfer(address payable recipient, uint256 amount) private {
         (bool success, ) = recipient.call{value: amount}("");
         require(success, "ETH transfer failed");
     }

     function redeemEth(uint256 amount) public onlyIfWeHaveEnoughReserves(amount) {
         // ...
         safeWethWithdraw(weth, amount);
         safeEthTransfer(payable(msg.sender), amount);
     }

Assessed type

Error

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #25

[raymondfam]

weth and dcntEth are ERC20 tokens not to be mixed up with native coin using call().

[c4-judge]

alex-ppg marked the issue as not a duplicate

[alex-ppg]

The Warden has confused low-level calls (that always yield a bool and a bytes memory variable) with interface based interactions that may yield nothing or yield different variables and whose execution is guaranteed to either succeed or revert the whole transaction.

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Invalid