Ondo Finance - pkqs90's results

Lines of code

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

Vulnerability details

Impact

When users call ousgInstantManager.sol#mintRebasingOUSG(), some of the rOUSG shares that are supposed to send to user is accmulated in ousgInstantManager.sol contract instead.

Bug Description

When performing mintRebasingOUSG() there are three steps:

1. Call internal _mint() to exchange USDC for OUSG
2. Wrap OUSG for rOUSG
3. Send rOUSG to user

The issue happens in step 3. Since rOUSG is a rebasing token, to avoid precision error, transferShare() should be used instead of transfer(). The original implementation would suffer from 2 precision errors during calculation, first is rOUSGShares -> rOUSG, second is rOUSG -> rOUSGShares (during rousg.transfer()). Both precision errors can be simply avoided if we use transferShare() in the first place.

  function mintRebasingOUSG(
    uint256 usdcAmountIn
  )
    external
    override
    nonReentrant
    whenMintNotPaused
    returns (uint256 rousgAmountOut)
  {
    uint256 ousgAmountOut = _mint(usdcAmountIn, address(this));
    ousg.approve(address(rousg), ousgAmountOut);
    rousg.wrap(ousgAmountOut);
@>  rousgAmountOut = rousg.getROUSGByShares(
@>    ousgAmountOut * OUSG_TO_ROUSG_SHARES_MULTIPLIER
@>  );
@>  rousg.transfer(msg.sender, rousgAmountOut);
    emit InstantMintRebasingOUSG(
      msg.sender,
      usdcAmountIn,
      ousgAmountOut,
      rousgAmountOut
    );
  }

  function getROUSGByShares(uint256 _shares) public view returns (uint256) {
    return
      (_shares * getOUSGPrice()) / (1e18 * OUSG_TO_ROUSG_SHARES_MULTIPLIER);
  }

Proof of Concept

Add the following code to mint.t.sol and run unit test. We can see after Alice calls mintRebasingOUSG(), there are still some shares of rOUSG left in ousgInstantManager.

  function test_mintRebasingOUSG() public {
    // Set oracle price to a 123.45e18.
    oracleCheckHarnessOUSG.setPrice(123456789012345678900);
    deal(address(USDC), alice, 100_000e6);
    vm.startPrank(alice);
    USDC.approve(address(ousgInstantManager), 100_000e6);
    ousgInstantManager.mintRebasingOUSG(100_000e6);
    vm.stopPrank();
    // rOUSGToken.sharesOf(address(ousgInstantManager))=47
    console.log("rOUSGToken.sharesOf(address(ousgInstantManager))=", rOUSGToken.sharesOf(address(ousgInstantManager)));
  }

Tools Used

Foundry

Recommended Mitigation Steps

-    rousg.transfer(msg.sender, rousgAmountOut);
+    rousg.transferShares(msg.sender, ousgAmountOut * OUSG_TO_ROUSG_SHARES_MULTIPLIER);

Assessed type

Token-Transfer