code-423n4 / 2024-03-ondo-finance-findings

5 stars 6 forks source link

Errors can occur when users redeem their assets. #331

Closed c4-bot-4 closed 6 months ago

c4-bot-4 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/ousgInstantManager.sol#L388-L456

Vulnerability details

Impact

Users can lost their funds because of the loss of the buidl.

Proof of Concept

https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/ousgInstantManager.sol#L388-L456

  function _redeem(
    uint256 ousgAmountIn
  ) internal returns (uint256 usdcAmountOut) {
    require(
      IERC20Metadata(address(usdc)).decimals() == 6,
      "OUSGInstantManager::_redeem: USDC decimals must be 6"
    );
    require(
      IERC20Metadata(address(buidl)).decimals() == 6,
      "OUSGInstantManager::_redeem: BUIDL decimals must be 6"
    );
    uint256 ousgPrice = getOUSGPrice();
    uint256 usdcAmountToRedeem = _getRedemptionAmount(ousgAmountIn, ousgPrice);

    require(
      usdcAmountToRedeem >= minimumRedemptionAmount,
      "OUSGInstantManager::_redeem: Redemption amount too small"
    );
    _checkAndUpdateInstantRedemptionLimit(usdcAmountToRedeem);

    if (address(investorBasedRateLimiter) != address(0)) {
      investorBasedRateLimiter.checkAndUpdateRedeemLimit(
        msg.sender,
        usdcAmountToRedeem
      );
    }

    uint256 usdcFees = _getInstantRedemptionFees(usdcAmountToRedeem);
    usdcAmountOut = usdcAmountToRedeem - usdcFees;
    require(
      usdcAmountOut > 0,
      "OUSGInstantManager::_redeem: redeem amount can't be zero"
    );

    ousg.burn(ousgAmountIn);

    uint256 usdcBalance = usdc.balanceOf(address(this));

    if (usdcAmountToRedeem >= minBUIDLRedeemAmount) {
      // amount of USDC needed is over minBUIDLRedeemAmount, do a BUIDL redemption
      // to cover the full amount
      _redeemBUIDL(usdcAmountToRedeem);
    } else if (usdcAmountToRedeem > usdcBalance) {
      // There isn't enough USDC held by this contract to cover the redemption,
      // so we perform a BUIDL redemption of BUIDL's minimum required amount.
      // The remaining amount of USDC will be held in the contract for future redemptions.
      _redeemBUIDL(minBUIDLRedeemAmount);
      emit MinimumBUIDLRedemption(
        msg.sender,
        minBUIDLRedeemAmount,
        usdcBalance + minBUIDLRedeemAmount - usdcAmountToRedeem
      );
    } else {
      // We have enough USDC sitting in this contract already, so use it
      // to cover the redemption and fees without redeeming more BUIDL.
      emit BUIDLRedemptionSkipped(
        msg.sender,
        usdcAmountToRedeem,
        usdcBalance - usdcAmountToRedeem
      );
    }

    if (usdcFees > 0) {
      usdc.transfer(feeReceiver, usdcFees);
    }
    emit RedeemFeesDeducted(msg.sender, feeReceiver, usdcFees, usdcAmountOut);

    usdc.transfer(msg.sender, usdcAmountOut);
  }

This code assumed the ousgInstantManager have the certain amount of the buidl token. But there is no mechanism to get the buidl token. So if protocol doesn't transfer the buidl tokens to the OUSGInstantManager, users can lost their funds.

Tools Used

Manual Review

Recommended Mitigation Steps

Add the mechanism to support the buidl tokens to the OUSGInstantManager contract.

Assessed type

Token-Transfer

c4-pre-sort commented 6 months ago

0xRobocop marked the issue as insufficient quality report

3docSec commented 6 months ago

Out of scope as per contest README:

  • We are aware that if all BUIDL has been redeemed from our contract, we would not be able to provide instant redemptions for OUSG or rOUSG
c4-judge commented 6 months ago

3docSec marked the issue as unsatisfactory: Out of scope