attacker can take over USDC by calling investWithToken/exitToken more than once

[hats-bug-reporter[bot]]

Github username: @0Ksecurity Twitter username: -- Submission hash (on-chain): 0x44a88142e9efbde958fac789573563bb263c5e5bd7e9fbd04a4ed5c21eb3e2ab Severity: high

Description: Description\ an attacker can get most of the USDC tokens that exist in the protocol by calling the investWithToken and exitWithToken more than once, this is possible because the origamiInvestment open the possibility to exit with toToken == oUSDCwhich this can be used to decrease totalReserve in repricingToken.sol without changing the totalSupply of the oUSDC. (more in attack scenario)

Attack Scenario\

-lets say Alice decide to deposit 1000 USDC into the origami vault by calling the InvestWithToken in OrigamiInvestmentVault contract,this function will mint oUSDC to this vault by calling the InvestWithToken in the origamiOtoken.sol and then mint ovTokens to alice by calling _issueSharesFromReserves in repricingToken.sol, lets take a look at the function in origami invest

 function investWithToken(
        InvestQuoteData calldata quoteData
    ) external override nonReentrant returns (
        uint256 investmentAmount
    ) {
        if (quoteData.fromTokenAmount == 0) revert CommonEventsAndErrors.ExpectedNonZero();
        if (!_isAllowed(msg.sender)) revert CommonEventsAndErrors.InvalidAccess();

        // If investing with the reserve token, pull them from the user
        // Otherwise pull the fromToken and use to invest in the underlying Origami Investment
        uint256 reservesAmount;
        if (quoteData.fromToken == reserveToken) {
            reservesAmount = quoteData.fromTokenAmount;
            IERC20(reserveToken).safeTransferFrom(msg.sender, address(this), reservesAmount);

            //@audit the else below will run if you set from token to asset address which is USDC:
        } else {

            InvestQuoteData memory underlyingQuoteData = abi.decode(
                quoteData.underlyingInvestmentQuoteData, (InvestQuoteData)
            );

            if (quoteData.fromToken != underlyingQuoteData.fromToken) revert CommonEventsAndErrors.InvalidToken(
                underlyingQuoteData.fromToken
            );
            if (quoteData.fromTokenAmount != underlyingQuoteData.fromTokenAmount) revert CommonEventsAndErrors.InvalidAmount(
                underlyingQuoteData.fromToken,
                underlyingQuoteData.fromTokenAmount
            );

            // Pull the `fromToken` into this contract and approve the reserveToken to pull it.
            IERC20(quoteData.fromToken).safeTransferFrom(msg.sender, address(this), quoteData.fromTokenAmount); // transfer USDC to this contract
            IERC20(quoteData.fromToken).safeIncreaseAllowance(reserveToken, quoteData.fromTokenAmount);

            reservesAmount = IOrigamiInvestment(reserveToken).investWithToken(underlyingQuoteData);
        }

        investmentAmount = _issueSharesFromReserves(
            reservesAmount,
            msg.sender,
            quoteData.minInvestmentAmount
        );

        emit Invested(msg.sender, quoteData.fromTokenAmount, quoteData.fromToken, investmentAmount);
    }

• this function will call the InvestWithToken inside the origamiOtoken.sol which will mint oUSDC to the vault:

 function investWithToken(
        InvestQuoteData calldata quoteData
    ) external virtual override nonReentrant returns (uint256 investmentAmount) {
        if (quoteData.fromTokenAmount == 0) revert CommonEventsAndErrors.ExpectedNonZero();

        // Send the investment token to the manager
        IOrigamiOTokenManager _manager = manager; //OrigamiLendingSupplyManager
        IERC20(quoteData.fromToken).safeTransferFrom(msg.sender, address(_manager), quoteData.fromTokenAmount); // transfer the USDC amount to the manger contract
        investmentAmount = _manager.investWithToken(msg.sender, quoteData); // call this to get the invest amount

        emit Invested(msg.sender, quoteData.fromTokenAmount, quoteData.fromToken, investmentAmount);

        // Mint the oToken for the user
        if (investmentAmount != 0) {
            _mint(msg.sender, investmentAmount); // oUSDC increase here
        }
    }

this function calls the _mint which increase the totalsupply of the oUSDC:

function _mint(address account, uint256 amount) internal virtual {
        require(account != address(0), "ERC20: mint to the zero address");

        _beforeTokenTransfer(address(0), account, amount);

        _totalSupply += amount;
        unchecked {
            // Overflow not possible: balance + amount is at most totalSupply + amount, which is checked above.
            _balances[account] += amount;
        }
        emit Transfer(address(0), account, amount);

        _afterTokenTransfer(address(0), account, amount);
    }

• now if we take a look we see that the _issueSharesFromReserves will mint ovUSDC depending on the totalSuuply of oUSDC and totalReserves which is the vesting amount + pending amount.

    function _issueSharesFromReserves(
        uint256 reserveTokenAmount,
        address recipient,
        uint256 minSharesAmount
    ) internal returns (uint256 sharesAmount) {
        sharesAmount = reservesToShares(reserveTokenAmount);
        if (sharesAmount == 0) revert CommonEventsAndErrors.ExpectedNonZero();
        if (sharesAmount < minSharesAmount) revert CommonEventsAndErrors.Slippage(minSharesAmount, sharesAmount);

        // Mint shares(ovUSDC) to the user and add to the total reserves
        //@audit mint before update state
        _mint(recipient, sharesAmount);

        vestedReserves += reserveTokenAmount;
        emit VestedReservesAdded(reserveTokenAmount);

        _validateReservesBalance();
    }

     function reservesToShares(uint256 reserves) public view override returns (uint256) {
        uint256 _totalSupply = totalSupply();

        // Returns shares = 1:1 if no shares yet allocated
        // Not worth having a special check for totalReserves=0, it can revert with a panic
        // Round down for calculating shares from reserves
        return (_totalSupply == 0)
            ? reserves
            : reserves.mulDiv(_totalSupply, totalReserves(), OrigamiMath.Rounding.ROUND_DOWN);
    }

• now alice after she gets ovTokens, she calls the exitToToken function and setting the toToken == reserveToken(oUSDC) and this will lead to execute the logic below:

    function exitToToken(
        ExitQuoteData calldata quoteData,
        address recipient
    ) external override returns (
        uint256 toTokenAmount
    ) {
        if (quoteData.investmentTokenAmount == 0) revert CommonEventsAndErrors.ExpectedNonZero();
        if (recipient == address(0)) revert CommonEventsAndErrors.InvalidAddress(recipient);

        // If exiting to the reserve token, redeem and send them to the user
        // Otherwise first redeem the reserve tokens and then exit the underlying Origami investment
        if (quoteData.toToken == reserveToken) {
            toTokenAmount = _redeemReservesFromShares(
                quoteData.investmentTokenAmount,
                msg.sender,
                quoteData.minToTokenAmount,
                recipient
            );

        }
        //... only the part above needed for this report (to make it simple for reading)
    }

        function _redeemReservesFromShares(
        uint256 sharesAmount,
        address from,
        uint256 minReserveTokenAmount,
        address receiver
    ) internal returns (uint256 reserveTokenAmount) {
        if (balanceOf(from) < sharesAmount) revert CommonEventsAndErrors.InsufficientBalance(address(this), sharesAmount, balanceOf(from));

        reserveTokenAmount = sharesToReserves(sharesAmount);
        if (reserveTokenAmount == 0) revert CommonEventsAndErrors.ExpectedNonZero();
        if (reserveTokenAmount < minReserveTokenAmount) revert CommonEventsAndErrors.Slippage(minReserveTokenAmount, reserveTokenAmount);

        // Burn the users shares and remove the amount of reserves from the vestedReserve
        //@audit burn ovUSDC
        _burn(from, sharesAmount);

        vestedReserves -= reserveTokenAmount;
        emit VestedReservesRemoved(reserveTokenAmount);
        if (receiver != address(this)) {
            IERC20(reserveToken).safeTransfer(receiver, reserveTokenAmount);
        }

        _validateReservesBalance();
    }

now, we can see that the vestedReserves will decrease with the reserveTokenAmount and in this case that alice asked for toToken to be equal to the oUSDC, there is no burn of the oUSDC which that mean the totalSupply did not increase. this will give alice more ovUSDC any time she repeat these steps, lets take an example:

• lets say the totalSupply and the vestedReserves both are 10,000 and alice decide to invest/deposit with 1000 USDC, when she calls the investWithToken this will happen : investWithToken --> investWithToken(inside the oUSDC vault) --> mint 1000 oUSDC to the investment vault --> call _issueSharesFromReserves which calculates the ovUSDC like this 1000(Invest amount) * 11000 / 1000 = 1100;

• now, after alice have 1100 of ovUSDC the vestedReserves is updated to 11000 too, now she will call the exitToken with quoteData.toToken == reserveToken(oUSDC) which do the follow: exitToken --> call _redeemReservesFromShares with investment share of 1100 ovUSDC --> get the invest amount 1100(ovUSDC) * 11000(totalReserve) / 11000 == 1100 and then burn 1100 from the reserve without doing any changes to the totalSupply of oUSDC:

_burn(from, sharesAmount);

        vestedReserves -= reserveTokenAmount; //@audit now the vestedReserve == 9900
        emit VestedReservesRemoved(reserveTokenAmount);
        if (receiver != address(this)) {
            IERC20(reserveToken).safeTransfer(receiver, reserveTokenAmount);
        }

alice repeat this step for second time:

get ovUSDC : 1100 USDC * 11000 / 9900 = 1222 ovUSDC

call exitToken toToken == reserveToken(oUSDC) : 1222 * 9900 / 11000 == 1099 decrease the vestedReserves which is totalReserve we use in calculating with 1099 which become 8801 :

 _burn(from, sharesAmount);

        vestedReserves -= reserveTokenAmount;
        emit VestedReservesRemoved(reserveTokenAmount);
        if (receiver != address(this)) {
            IERC20(reserveToken).safeTransfer(receiver, reserveTokenAmount);
        }

this step can be repeated till the totalReserve decrease to favorable amount for alice while totalsupply is the same and this lead to mint and then redeem most of the USDC inside the protocol.

note: in some point of the scenario above we didn't calculated round down + fee but the attack still in alice favour when the protocol have more USDC.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional) recommend to update state of the vestedReserves before the calculation and manage the totalSupply of the oUSDC in case when users get oUSDC to do invest or when they exit to avoid scenario above.

Files:

• RepricingToken.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmXiLiZiZ9HXhErkv2iB3bZjAFaSJVyPNVuZz8KRSh9BuG)
• OrigamiInvestmentVault.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmQPFUQftvL7eTeY8wEbMVLebBfZx1agaUCtBGSwDxdvex)
• OrigamiOToken.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmboDJgi4YcPnz6d5eAZVCbj5NUKUwVQ4fqrnk4i9nTGB9)
• OrigamiLendingSupplyManager.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmQuA2GSeirYeGC5k6d4mTxYjiiDUvsJpsncP8smRomR6o)

[frontier159]

Please provide a PoC with your findings -- the descriptions above are mixing the use of ovUSDC and oUSDC, and it's not clear where the exploit is that you suggest.

• ovUSDC is an instance of OrigamiInvestmentVault: OrigamiInvestmentVault::investWithToken() will mint new ovUSDC tokens
• oUSDC is an instance of OrigamiOToken: OrigamiOToken::investWithToken() will mint new oUSDC tokens

Calling OrigamiInvestmentVault::investWithToken() does not mint oUSDC tokens

[frontier159]

I think you are conflating the totalSupply() between ovUSDC and oUSDC.

This example isn't correct:

lets say the totalSupply and the vestedReserves both are 10,000 and alice decide to invest/deposit with 1000 USDC, when she calls the investWithToken this will happen : investWithToken --> investWithToken(inside the oUSDC vault) --> mint 1000 oUSDC to the investment vault --> call _issueSharesFromReserves which calculates the ovUSDC like this 1000(Invest amount) * 11000 / 1000 = 1100;

Using the starting amounts for totalSupply() and totalReserves() from your example

ovUSDC.totalSupply() == 10_000
ovUSDC.totalReserves() == 10_000
ovUSDC.reservesPerShare() == 1e18

ovUSDC.investWithToken(1_000 USDC):
  - oUSDC.investWithToken(1_000 USDC) ==> 1000 oUSDC minted
      - oUSDC.totalSupply() += 1_000
  - new ovUSDC shares == 1_000 * ovUSDC.totalSupply() / ovUSDC.totalReserves()
                      == 1_000 * 10_000 / 10_000
                      == 1_000
  - 1_000 new ovUSDC shares minted to alice
  - 1_000 oUSDC added as vested reserves to ovUSDC.
  - ovUSDC.totalSupply() == 11_000
  - ovUSDC.totalReserves() == 11_000
  - ovUSDC.reservesPerShare() == 1e18 (the same)

[0Ksecurity]

hey frontier159, thanks for response.

while the totalSupply is not for the oUSDC then the scenario we mentioned is impossible to happen.

thanks for details you provide.