Balance check in _redeemBUIDL() can cause redeem() to fail

[c4-bot-10]

Lines of code

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/ousg/ousgInstantManager.sol#L458

Vulnerability details

Impact

When swapping BUIDL for USDC in _redeemBUIDL(), balance after check can potentially cause function to revert.

Proof of Concept

Users are able to redeem() OUSG for USDC. In cases where OUSGInstantManager.sol does not hold enough USDC, BUIDL tokens are swapped for USDC.

To source the USDC liquidity, this contract will hold the BUIDL token and interact with a currently unknown third party contract that swaps BUIDL for USDC. Unfortunately, the third party BUIDL-to-USDC swap functionality is not yet verified or confirmed as this time, but for the sake of this audit it can be assumed to be correct. The interface that this contract uses for BUIDL-to-USDC swaps has been inferred from public transactions on Etherscan. 1 BUIDL can always be assumed to be worth 1 USDC.

    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
      );
    }

Within _redeemBUIDL(), the function keeps track of usdcBalanceBefore to ensure that the balance afterward equals usdcBalanceBefore + buidlAmountToRedeem. However, this check poses a significant risk as it could potentially cause the entire function to revert. While the third party contract that swaps BUIDL for USDC is unknown. It is highly improbable for any liquidity pool to maintain a precisely 1:1 ratio for USDC/BUIDL. Consequently, even a minor discrepancy, such as a difference of 1 wei in the swap amount, could lead to the failure of the function.

Tools Used

Manual Review

Recommended Mitigation Steps

_redeemBUIDL() should make sure balance after is equal or greater than usdcBalanceBefore + buidlAmountToRedeem

  function _redeemBUIDL(uint256 buidlAmountToRedeem) internal {
    require(
      buidl.balanceOf(address(this)) >= minBUIDLRedeemAmount,
      "OUSGInstantManager::_redeemBUIDL: Insufficient BUIDL balance"
    );
    uint256 usdcBalanceBefore = usdc.balanceOf(address(this));
    buidl.approve(address(buidlRedeemer), buidlAmountToRedeem);
    buidlRedeemer.redeem(buidlAmountToRedeem);
    require(
-     usdc.balanceOf(address(this)) == usdcBalanceBefore + buidlAmountToRedeem,
+     usdc.balanceOf(address(this)) >= usdcBalanceBefore + buidlAmountToRedeem,
      "OUSGInstantManager::_redeemBUIDL: BUIDL:USDC not 1:1"
    );
  }

Assessed type

DoS

[0xRobocop]

From the docs, it should be assumed that BUIDL redeem contract is "correct" so we can assume the redeem is 1:1.

I will leave for sponsor review.

[c4-pre-sort]

0xRobocop marked the issue as sufficient quality report

[c4-pre-sort]

0xRobocop marked the issue as primary issue

[cameronclifton]

Agree with " it should be assumed that BUIDL redeem contract is "correct" so we can assume the redeem is 1:1."

[c4-sponsor]

cameronclifton (sponsor) disputed

[3docSec]

Looks like a good candidate for an analysis report. As the redeemer code is not public, we can't really prove that a BUIDL depeg is possible, so this finding doesn't hold the burden of proof.

[c4-judge]

3docSec marked the issue as unsatisfactory:
Insufficient proof