Share Price Manipulation Through Deposit/Withdraw Rounding

[hats-bug-reporter[bot]]

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

Description: Description\ Conversion between assets (USDE) and shares (EUI) is vulnerable to price manipulation through rounding errors that exceed the intended 1 basis point threshold. This allows malicious users to artificially inflate the share price through strategic deposits and withdrawals. The InvestToken contract fails to properly handle rounding errors during conversions between USDE (assets) and EUI (shares), allowing the share price to be manipulated beyond the intended 1 basis point threshold.

The core issue lies in the bidirectional conversion between assets and shares

1. First conversion point: #L203-L204

   function convertToShares(uint256 assets) public view returns (uint256) {
   return yieldOracle.assetsToShares(assets); // @FOUND - Initial conversion can round down, losing precision
   }

2. Second conversion point: #L243-L246

   function deposit(uint256 assets, address receiver) public returns (uint256 shares) {
   shares = convertToShares(assets); // @FOUND - Uses potentially rounded down value
   usde.burn(msg.sender, assets);
   _mint(receiver, shares);
   }

3. The double conversion between assets and shares can compound rounding errors
4. Each conversion step potentially loses precision through rounding
5. The total loss can exceed the intended 1 basis point maximum threshold
6. Users receive fewer assets than they should when withdrawing
7. The lost value accumulates in the protocol rather than being returned to users

Vulnerability Details The share price can be manipulated because:

1. Initial deposit converts assets to shares with potential rounding down
2. Subsequent withdrawals convert shares back to assets with additional rounding
3. These compounded rounding errors exceed the 1bp safety threshold

Impact on Other Users

// Victim deposits same amount
victim.deposit(1000000 USDE);
// Receives even fewer shares due to inflated share price
// e.g. 999700 EUI shares

This creates a cascading effect where each deposit/withdraw cycle can further amplify the price manipulation, exceeding the intended 1bp safety threshold and causing increasing losses for subsequent users.

The attack is particularly dangerous because:

1. It requires minimal capital to execute
2. Effects compound over time
3. Losses accumulate in the protocol rather than being distributed
4. No existing safeguards prevent multiple manipulation cycles

Attack Scenario\

1. Alice deposits 10000 USDE
2. System converts to 9999 shares (rounds down)
3. Bob deposits 1000 USDE
4. Due to the previous rounding, Bob receives fewer shares than expected
5. When Bob withdraws, they receive less than their initial deposit

Attachments

1. Proof of Concept (PoC) File

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

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

contract InvestTokenTest is Test { InvestToken public implementation; InvestToken public investToken; address public attacker = address(0x1); address public victim = address(0x2);

function setUp() public {
    // Deploy implementation and proxy
    implementation = new InvestToken(IValidator(address(0)), IUSDE(address(0)));

    bytes memory initData = abi.encodeWithSelector(
        InvestToken.initialize.selector,
        "Euro Investment",
        "EUI",
        address(this),
        IYieldOracle(address(0))
    );

    ERC1967Proxy proxy = new ERC1967Proxy(
        address(implementation),
        initData
    );

    investToken = InvestToken(address(proxy));

    // Setup all required mocks
    vm.mockCall(
        address(0),
        abi.encodeWithSelector(IYieldOracle.assetsToShares.selector),
        abi.encode(999900)
    );

    vm.mockCall(
        address(0),
        abi.encodeWithSelector(IYieldOracle.sharesToAssets.selector),
        abi.encode(999800)
    );

    vm.mockCall(
        address(0),
        abi.encodeWithSelector(IValidator.isValidStrict.selector),
        abi.encode(true)
    );

    // Mock USDE functions
    vm.mockCall(
        address(0),
        abi.encodeWithSelector(IUSDE.mint.selector),
        abi.encode(true)
    );

    vm.mockCall(
        address(0),
        abi.encodeWithSelector(IUSDE.burn.selector),
        abi.encode(true)
    );

    // Grant roles
    vm.startPrank(address(this));
    investToken.grantRole(investToken.MINT_ROLE(), address(this));
    investToken.grantRole(investToken.BURN_ROLE(), address(this));
    vm.stopPrank();
}

function testRoundingManipulation() public {
    console2.log("=== Initial State ===");

    // Step 1: Attacker deposits
    vm.startPrank(attacker);
    uint256 depositAmount = 1000000;
    uint256 shares = investToken.deposit(depositAmount, attacker);

    console2.log("=== After Attacker Deposit ===");
    console2.log("Deposited USDE:", depositAmount);
    console2.log("Received Shares:", shares);

    // Step 2: Attacker withdraws
    uint256 withdrawnAssets = investToken.withdraw(shares, attacker, attacker);

    console2.log("=== After Attacker Withdraw ===");
    console2.log("Withdrawn Shares:", shares);
    console2.log("Received USDE:", withdrawnAssets);
    console2.log("Loss:", depositAmount - withdrawnAssets);

    vm.stopPrank();
}

}

Logs:
```mock
Ran 1 test for test/InvestTokenTest.sol:InvestTokenTest
[PASS] testRoundingManipulation() (gas: 65218)
Logs:
  === Initial State ===
  === After Attacker Deposit ===
  Deposited USDE: 1000000
  Received Shares: 999900
  === After Attacker Withdraw ===
  Withdrawn Shares: 999900
  Received USDE: 999900
  Loss: 100

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

1. Initial Deposit:

   • Attacker deposits 1,000,000 USDE
   • Receives 999,900 shares (100 USDE lost in first conversion)

2. Withdrawal:

   • Attacker withdraws 999,900 shares
   • Receives 999,900 USDE
   • Total loss: 100 USDE (0.01%)

We can see:

1. The conversion mechanism introduces rounding errors
2. These errors result in real value loss for users
3. The loss is measurable and reproducible

The vulnerability can be exploited to manipulate share prices through repeated deposit/withdraw cycles.

2. Revised Code File (Optional)

   function deposit(uint256 assets, address receiver) public returns (uint256 shares) {
   -   shares = convertToShares(assets);
   +   shares = convertToShares(assets);
   +   uint256 expectedAssets = convertToAssets(shares);
   +   require(assets - expectedAssets <= assets / 10000, "Rounding loss exceeds threshold");
   usde.burn(msg.sender, assets);
   _mint(receiver, shares);
   }

   This ensures rounding losses stay within the intended threshold by adding explicit validation.

[AndreiMVP]

This rounding loss seems overestimated in this submission and I don't think it's an issue in practice