Mitigation Confirmed for NEW

code423n4 commented 1 year ago

Note: Issue has not actually been resolved but for some reason I can't get my issues to submit without "Mitigation confirmed (no new vulnerabilities detected)" checked so I am doing this as a work around

Severity

Medium

Lines of code

https://github.com/asymmetryfinance/smart-contracts/pull/258/files#diff-6abc8f2e4ad1647a12784e9fbf18e9c5f86c05668e3e89e2a51ab569992b214fR92-R108

Impact

User will receive less than they should on larger withdrawal due to all or nothing nature of withdrawal check

Proof of Concept

    if (canWithdrawFromRocketPool(_amount)) {
        RocketTokenRETHInterface(rethAddress()).burn(_amount);
    } else {
        uint256 wethBalanceBefore = IERC20(W_ETH_ADDRESS).balanceOf(
            address(this)
        );
        uint256 ethPerReth = ethPerDerivative();
        uint256 minOut = ((((ethPerReth * _amount) / 10 ** 18) *
            ((10 ** 18 - maxSlippage))) / 10 ** 18);
        uint256 idealOut = ((((ethPerReth * _amount) / 10 ** 18) *
            ((10 ** 18))) / 10 ** 18);
        IERC20(rethAddress()).approve(ROCKET_SWAP_ROUTER, _amount);

        // swaps from reth into weth using 100% balancer pool
        RocketSwapRouterInterface(ROCKET_SWAP_ROUTER).swapFrom(
            0,
            10,
            minOut,
            idealOut,
            _amount
        );
        uint256 wethBalanceAfter = IERC20(W_ETH_ADDRESS).balanceOf(
            address(this)
        );
        IWETH(W_ETH_ADDRESS).withdraw(wethBalanceAfter - wethBalanceBefore);
    }

Reth#withdraw has been changed to first check if the full amount being withdrawn can instead be burned directly from the rocketPool contract. This issue with this is that it creates an all or nothing withdrawal which is leaks value.

When redeeming directly from the rocketPool contract the user will always receive the exact exchange ratio of reth. However when swapping the user will have to pay a swap fee which will directly reduce the amount of funds they will receive from the withdrawal.

This is problematic if a withdrawal can be partially handled by the rocketPool. In a situation like this, the user will get a suboptimal rate because they will swap the entire withdrawal amount instead withdrawing as much as possible from rocketPool first.

Example: Assume RETH = 1 ETH, both via swap and the internal exchange rate. Currently 10 ETH can be withdrawn from the rocketPool contract. A user attempts to withdraw 11 ETH. The contract will now swap all 11 ETH, paying as swap fee on all of them. This results in the user receiving the following assuming a 0.3% swap fee:

11 1 0.997 = 10.967

Instead if the contract used a hybrid approach they user would net more. First it should withdraw all 10 ETH from the rocketPool and only swap the remaining 1 ETH:

10 1 + 1 1 * 0.997 = 10.997