ERC4626 Share Price Manipulation Through Rounding Errors in Conversion Functions

[hats-bug-reporter[bot]]

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

Description: Description\ InvestToken implementation allows for share price manipulation through rounding errors in the asset/share conversion functions. This can lead to users receiving more assets than initially deposited through careful manipulation of deposit/withdraw cycles.

The conversion functions rely entirely on the yieldOracle for price conversions: InvestToken.sol#L203-L213

    function convertToShares(uint256 assets) public view returns (uint256) {
        // @FOUND - Direct passthrough to oracle without rounding control
        // This allows potential value inflation through roundtrip conversions
        return yieldOracle.assetsToShares(assets);
       // @FOUND - Uncontrolled rounding in conversion allows share price manipulation
    }

    function convertToAssets(uint256 shares) public view returns (uint256) {
        // @FOUND - Direct passthrough to oracle without rounding control
        // Combined with convertToShares, this enables roundtrip inflation
        return yieldOracle.sharesToAssets(shares);
       // @FOUND - Return value can be inflated through roundtrip conversions
    }

The issue lies in the lack of rounding direction enforcement. When converting assets->shares->assets, rounding errors could accumulate leading to a violation where the final assets value is greater than the input amount.

This is dangerous because:

1. The conversion functions delegate all rounding behavior to the external oracle without enforcing rounding direction
2. When used in sequence (assets -> shares -> assets), rounding errors can accumulate leading to value inflation
3. This violates a core ERC4626 invariant that roundtrip conversions must never create value
4. This issue affects all deposit/withdraw operations since they rely on these conversion functions, and
5. malicious users could potentially exploit this to extract more assets than initially deposited through careful manipulation of deposit/withdraw amounts

This vulnerability becomes particularly critical in high-frequency trading scenarios or when dealing with large amounts where even small rounding discrepancies can lead to significant value extraction, because the issue directly impacts protocol solvency since it allows users to receive more assets than they should be entitled to based on their share position.

Attack Scenario\

1. Initial state: Share price = 1 USDE/share
2. Attacker deposits 1000 USDE, receives 1000 shares
3. Attacker performs multiple small deposits/withdrawals to exploit rounding

   
   // Round 1
   deposit(100 USDE) -> 99.9 shares
   withdraw(99.9 shares) -> 100.1 USDE

// Round 2 deposit(100.1 USDE) -> 100 shares withdraw(100 shares) -> 100.2 USDE

4. After multiple rounds, attacker withdraws all shares with inflated value
5. Result: Attacker extracts more USDE than initially deposited

**Attachments**

1. **Proof of Concept (PoC) File**
As we see above, the InvestToken contract implements ERC4626 with conversion functions that rely on an external yield oracle for price calculations. The vulnerability lies in the uncontrolled rounding behavior during asset-share conversions, which can be exploited to manipulate the effective share price.

To demonstrate the rounding vulnerability.
```solidity
// 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 {IValidator} from "../src/interfaces/IValidator.sol";
import {IYieldOracle} from "../src/interfaces/IYieldOracle.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";

contract MockYieldOracle is IYieldOracle {
    function assetsToShares(uint256 assets) external pure returns (uint256) {
        return assets + 1; // Intentionally inflate shares
    }

    function sharesToAssets(uint256 shares) external pure returns (uint256) {
        return shares + 1; // Intentionally inflate assets
    }
}

contract MockValidator is IValidator {
    function isValidStrict(address, address) external pure returns (bool) {
        return true;
    }

    function isValid(address, address) external pure returns (bool) {
        return true;
    }
}

contract InvestTokenExploitTest is Test {
    InvestToken public token;
    MockYieldOracle public oracle;
    MockValidator public validator;

    function setUp() public {
        // Deploy mocks
        oracle = new MockYieldOracle();
        validator = new MockValidator();

        // Deploy implementation
        InvestToken implementation = new InvestToken(IValidator(address(validator)), IUSDE(address(this)));

        // Deploy proxy
        bytes memory initData = abi.encodeWithSelector(
            InvestToken.initialize.selector,
            "Test",
            "TST",
            address(this),
            IYieldOracle(address(oracle))
        );

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

        token = InvestToken(address(proxy));
    }

    function testRoundingExploit() public view {
        uint256 initialAssets = 1000e18;
        uint256 shares = token.convertToShares(initialAssets);
        uint256 finalAssets = token.convertToAssets(shares);

        console2.log("Initial assets:", initialAssets);
        console2.log("Shares received:", shares);
        console2.log("Final assets:", finalAssets);
        console2.log("Value extracted:", finalAssets - initialAssets);

        assert(finalAssets > initialAssets);
    }
}

Logs

Ran 1 test for test/InvestTokenExploitTest.sol:InvestTokenExploitTest
[PASS] testRoundingExploit() (gas: 23293)
Logs:
  Initial assets: 1000000000000000000000
  Shares received: 1000000000000000000001
  Final assets: 1000000000000000000002
  Value extracted: 2

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

2. Revised Code File (Optional)

   
   function convertToShares(uint256 assets) public view returns (uint256) {
   -   return yieldOracle.assetsToShares(assets);
   +  // Round down shares to prevent share inflation
   +   uint256 shares = yieldOracle.assetsToShares(assets);
   +   return shares.roundDown(); // Ensure rounding favors protocol
   }

function convertToAssets(uint256 shares) public view returns (uint256) {

• return yieldOracle.sharesToAssets(shares);
• // Round down assets to prevent value extraction
• uint256 assets = yieldOracle.sharesToAssets(shares);
• return assets.roundDown(); // Prevent inflation on withdrawals }

  
  This fix implements conservative rounding that favors the protocol, preventing value extraction through conversion manipulation while maintaining ERC4626 vault invariants.

[AndreiMVP]

Rounding might not be perfect but in practice it's not an issue since the difference would be too insignificant to exploit.