search

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

5 stars 2 forks source link

BathToken stake holders may not be able to `withdraw` as `outstandingAmount` can not be liquidated permissionless #348

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L597-L605

Vulnerability details

    uint256 r = (underlyingBalance().mul(_shares)).div(_initialTotalSupply);
    _burn(msg.sender, _shares);
    uint256 _fee = r.mul(feeBPS).div(10000);
    // If FeeTo == address(0) then the fee is effectively accrued by the pool
    if (feeTo != address(0)) {
        underlyingToken.transfer(feeTo, _fee);
    }
    amountWithdrawn = r.sub(_fee);
    underlyingToken.transfer(receiver, amountWithdrawn);

https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L756-L759

function underlyingBalance() public view returns (uint256) {
    uint256 _pool = IERC20(underlyingToken).balanceOf(address(this));
    return _pool.add(outstandingAmount);
}

In the current implementation, totalAssets is IERC20(underlyingToken).balanceOf(address(this)) + outstandingAmount.

However, when there are some outstandingAmount, the actual redeemable amount of underlyingToken is less than underlyingBalance().

As a result, when a user tries to redeem a larger amounts of shares, the transaction can revert due to insufficient balance.

PoC

Given:

  1. Alice deposit 10,000 USDT to BathTokenA Pool, got 10,000 * 1e18 shares
  1. Strategist call placeMarketMakingTrades on Pair, place a offer: pay_amt = 1,000 1e18, buy_gem = USDC, buy_amt = 1,000 1e6
  1. Alice try redeem 9,500 * 1e18 shares, the transaction will revert.

Recommendation

Consider allowing the caller to trigger order cancelling when the balance in the contract is not enough for the withdrawal.

Furthermore, consider find a way to make the rebalancing permissionless so that the stake holders can always get back what they are owned without any centrialized roles.

bghughes commented 2 years ago

I would argue this is intended functionality. See EIP 4266 which tries to formalize this some. It is known that only the Reserve Ratio (%) of the pool will be available.

HickupHH3 commented 2 years ago

Agree with sponsor in this case. The user can at least redeem some shares if he wanted to. Would be a bank run scenario. Downgrading to QA.

HickupHH3 commented 2 years ago

Warden doesn't have QA report.