Lines of code

https://eips.ethereum.org/EIPS/eip-4626#previewwithdraw https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathToken.sol#L505

Vulnerability details

Impact

Revert of withdraw transactions.

Proof of Concept

From previewWithdraw (link1):

uint256 amountWithdrawn;
            uint256 _fee = assets.mul(feeBPS).div(10000);
            amountWithdrawn = assets.sub(_fee);
            shares = convertToShares(amountWithdrawn);

The operations the function does is to calculate the fee that should be payed when withdrawing this amount (aasets) And subtract it from the amount. Then convert it to the equivalent amount of shares and return it. So now if we'll withdraw those shares ,of course we'll receive an amount that <= assets (because those shares are equivalent to assets- fee and withdraw also takes a fee).

Now let's look at the withdraw implementaion (link2):

require(
            owner == msg.sender,
            "This implementation does not support non-sender owners from withdrawing user shares"
        );
        uint256 expectedShares = previewWithdraw(assets);
        uint256 assetsReceived = _withdraw(expectedShares, receiver);
        require(
            assetsReceived >= assets,
            "You cannot withdraw the amount of assets you expected"
        );
        shares = expectedShares;

The functions uses previewWithdraw on assets and then call _withdraw with the amount of shares the previewWithdraw returned. As we mentioned before _withdraw would return an amount that <= assets, so now the next retire would fail:

require(
            assetsReceived >= assets,
            "You cannot withdraw the amount of assets you expected"
        );

(because assetsReccive is always <= assets

Tools Used

Recommended Mitigation Steps

I thinks that previewWithdraw should return :converToShares(x/(1-feeBps)) instead of the current implementation.

code-423n4 / 2022-05-rubicon-findings