Anyone can withdraw the cryptocurrency in the Router

[code423n4]

Lines of code

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

Vulnerability details

Impact

Router.unwrapWETH9/refundETH/sweepToken is used to withdraw WETH/ETH/ERC20 tokens in the Router contract, but these three functions do not implement access control, which allows anyone to call these three functions to withdraw the cryptocurrency in the Router contract　

    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);
    }

Note: In Router.removeLiquidity/exactOutputInternal, when recipient == 0, tokens will be sent to the Router contract

    function exactInputInternal(uint256 amountIn, address recipient, uint256 sqrtPriceLimitD18, SwapCallbackData memory data) private returns (uint256 amountOut) {
        if (recipient == address(0)) recipient = address(this);
...
    function removeLiquidity(
        IPool pool,
        address recipient,
        uint256 tokenId,
        IPool.RemoveLiquidityParams[] calldata params,
        uint256 minTokenAAmount,
        uint256 minTokenBAmount,
        uint256 deadline
    ) external checkDeadline(deadline) returns (uint256 tokenAAmount, uint256 tokenBAmount, IPool.BinDelta[] memory binDeltas) {
        require(msg.sender == position.ownerOf(tokenId), "P");

        if (recipient == address(0)) recipient = address(this);

Proof of Concept

https://github.com/code-423n4/2022-12-Stealth-Project/blob/fc8589d7d8c1d8488fd97ccc46e1ff11c8426ac2/router-v1/contracts/Router.sol#L59-L82 https://github.com/code-423n4/2022-12-Stealth-Project/blob/fc8589d7d8c1d8488fd97ccc46e1ff11c8426ac2/router-v1/contracts/Router.sol#L295-L306 https://github.com/code-423n4/2022-12-Stealth-Project/blob/fc8589d7d8c1d8488fd97ccc46e1ff11c8426ac2/router-v1/contracts/Router.sol#L121-L122

Tools Used

None

Recommended Mitigation Steps

Consider adding access control to Router.unwrapWETH9/refundETH/sweepToken

[Jeiwan]

This is expected behavior. The contract is designed to never store funds. However, since it's also designed to receive intermediate funds during multi-pool swaps: https://github.com/code-423n4/2022-12-Stealth-Project/blob/fc8589d7d8c1d8488fd97ccc46e1ff11c8426ac2/router-v1/contracts/Router.sol#L151 And it can be a recipient of funds: https://github.com/code-423n4/2022-12-Stealth-Project/blob/fc8589d7d8c1d8488fd97ccc46e1ff11c8426ac2/router-v1/contracts/Router.sol#L170 The functions mentioned in the report are intentionally made public so that users could withdraw any leftovers from the contract after swaps.

Also, the Router contract is a permissionless contract that doesn't implement access control, so there's no way to identify who will be the owner of tokens accumulated in the contract.

[c4-judge]

kirk-baird marked the issue as primary issue

[gte620v]

As @Jeiwan said, this is the expected behavior and is not a bug.

[c4-sponsor]

gte620v marked the issue as sponsor disputed

[c4-judge]

kirk-baird marked the issue as nullified

[kirk-baird]

This is intended functionality of the router and therefore I'm going to nullify these issues. Furthermore, it is expected the users of the router to sweep tokens at the end of a call.