c4-bot-7 commented 3 months ago

Lines of code

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

Vulnerability details

Impact

_checkAndUpdateInstantMintLimit() and _checkAndUpdateInstantRedemptionLimit() include fees, which could lead to a potential reduction in allowable amounts by up to 1.99% in each window.

Proof of Concept

Users have the ability to instant mint() and redeem() OUSG in exchange for USDC. _checkAndUpdateInstantMintLimit() and _checkAndUpdateInstantRedemptionLimit() are responsible for limiting the amount permitted in the current window.

  function _checkAndUpdateInstantMintLimit(uint256 amount) internal {
    require(amount > 0, "RateLimit: mint amount can't be zero");

    if (
      block.timestamp >= lastResetInstantMintTime + resetInstantMintDuration
    ) {
      // time has passed, reset
      currentInstantMintAmount = 0;
      lastResetInstantMintTime = block.timestamp;
    }
    require(
      amount <= instantMintLimit - currentInstantMintAmount,
      "RateLimit: Mint exceeds rate limit"
    );

    currentInstantMintAmount += amount;
  }

  function _checkAndUpdateInstantRedemptionLimit(uint256 amount) internal {
    require(amount > 0, "RateLimit: redemption amount can't be zero");

    if (
      block.timestamp >=
      lastResetInstantRedemptionTime + resetInstantRedemptionDuration
    ) {
      // time has passed, reset
      currentInstantRedemptionAmount = 0;
      lastResetInstantRedemptionTime = block.timestamp;
    }
    require(
      amount <= instantRedemptionLimit - currentInstantRedemptionAmount,
      "RateLimit: Redemption exceeds rate limit"
    );
    currentInstantRedemptionAmount += amount;
  }

In both _mint() and _redeem(), the protocol has the option to impose fees. These fees are deducted from usdcAmountIn and usdcAmountToRedeem respectively in each function. However, these fees are factored into _checkAndUpdateInstantMintLimit() and _checkAndUpdateInstantRedemptionLimit(), potentially resulting in a reduction of the allowable amount in each window by up to 1.99%.

Tools Used

Manual Review

Recommended Mitigation Steps

  function _mint(
    uint256 usdcAmountIn,
    address to
  ) internal returns (uint256 ousgAmountOut) {
    require(
      IERC20Metadata(address(usdc)).decimals() == 6,
      "OUSGInstantManager::_mint: USDC decimals must be 6"
    );
    require(
      usdcAmountIn >= minimumDepositAmount,
      "OUSGInstantManager::_mint: Deposit amount too small"
    );
-   _checkAndUpdateInstantMintLimit(usdcAmountIn);
    if (address(investorBasedRateLimiter) != address(0)) {
      investorBasedRateLimiter.checkAndUpdateMintLimit(
        msg.sender,
        usdcAmountIn
      );
    }

    require(
      usdc.allowance(msg.sender, address(this)) >= usdcAmountIn,
      "OUSGInstantManager::_mint: Allowance must be given to OUSGInstantManager"
    );

    uint256 usdcfees = _getInstantMintFees(usdcAmountIn);
    uint256 usdcAmountAfterFee = usdcAmountIn - usdcfees;
+   _checkAndUpdateInstantMintLimit(usdcAmountAfterFee);

    uint256 ousgPrice = getOUSGPrice(); 
    ousgAmountOut = _getMintAmount(usdcAmountAfterFee, ousgPrice);

    require(
      ousgAmountOut > 0,
      "OUSGInstantManager::_mint: net mint amount can't be zero"
    );

    if (usdcfees > 0) {
      usdc.transferFrom(msg.sender, feeReceiver, usdcfees);
    }
    usdc.transferFrom(msg.sender, usdcReceiver, usdcAmountAfterFee);

    emit MintFeesDeducted(msg.sender, feeReceiver, usdcfees, usdcAmountIn);

    ousg.mint(to, ousgAmountOut);
  }

  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;
+   _checkAndUpdateInstantRedemptionLimit(usdcAmountOut);
    require(
      usdcAmountOut > 0,
      "OUSGInstantManager::_redeem: redeem amount can't be zero"
    );

    ousg.burn(ousgAmountIn);

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

    if (usdcAmountToRedeem >= minBUIDLRedeemAmount) {
      _redeemBUIDL(usdcAmountToRedeem);
    } else if (usdcAmountToRedeem > usdcBalance) {
      _redeemBUIDL(minBUIDLRedeemAmount);
      emit MinimumBUIDLRedemption(
        msg.sender,
        minBUIDLRedeemAmount,
        usdcBalance + minBUIDLRedeemAmount - usdcAmountToRedeem
      );
    } else {
      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);
  }

Assessed type

Context

c4-pre-sort commented 3 months ago

0xRobocop marked the issue as duplicate of #47

c4-judge commented 3 months ago

3docSec changed the severity to QA (Quality Assurance)