code-423n4 / 2022-12-Stealth-Project-findings

0 stars 0 forks source link

Lack of access control for sweepToken, refundETH, unwrapWETH9 in Router.sol #54

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-Stealth-Project/blob/fc8589d7d8c1d8488fd97ccc46e1ff11c8426ac2/router-v1/contracts/Router.sol#L59 https://github.com/code-423n4/2022-12-Stealth-Project/blob/fc8589d7d8c1d8488fd97ccc46e1ff11c8426ac2/router-v1/contracts/Router.sol#L70 https://github.com/code-423n4/2022-12-Stealth-Project/blob/fc8589d7d8c1d8488fd97ccc46e1ff11c8426ac2/router-v1/contracts/Router.sol#L80

Vulnerability details

Impact

Lack of access control for sweepToken, refundETH, unwrapWETH9 in Router.sol, any WETH token, ETH and ERC20 sent to Router.sol is lost and claimable to anyone.

Proof of Concept

the function unwrapWETH9 and sweepToken and refundETH has no access control and can be called by anyone to sweep the token.

    /// @inheritdoc IRouter
    function unwrapWETH9(uint256 amountMinimum, address recipient) public payable override {
        uint256 balanceWETH9 = WETH9.balanceOf(address(this));
        require(balanceWETH9 >= amountMinimum, "Insufficient WETH9");

        if (balanceWETH9 > 0) {
            WETH9.withdraw(balanceWETH9);
            TransferHelper.safeTransferETH(recipient, balanceWETH9);
        }
    }

    /// @inheritdoc IRouter
    function sweepToken(IERC20 token, uint256 amountMinimum, address recipient) public payable {
        uint256 balanceToken = token.balanceOf(address(this));
        require(balanceToken >= amountMinimum, "Insufficient token");

        if (balanceToken > 0) {
            TransferHelper.safeTransfer(address(token), recipient, balanceToken);
        }
    }

    /// @inheritdoc IRouter
    function refundETH() external payable override {
        if (address(this).balance > 0) TransferHelper.safeTransferETH(msg.sender, address(this).balance);
    }

consider this case:

  1. Alice accidentally sent 100 USDC to the router.
  2. Alice wants to get the USDC back.
  3. Some one call sweepToken and set recipient as himself to get the token.
  4. The token should belong to Alice, but she cannot get the token.

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend project only let admin sweep the token to destinated address and refund and resuce the asset. We also recommend the project refund any exessive ETH sent by user if the function is marked as payable.

c4-judge commented 1 year ago

kirk-baird marked the issue as duplicate of #30

c4-judge commented 1 year ago

kirk-baird marked the issue as nullified