hats-finance / Euro-Dollar-0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd

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

Any user who deposits and withdraws within the same price window will lose funds due to this price mismatch between deposit and withdrawal calculations. #107

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): 0xc901f42bbdef87cd0f58618bc21c318da98a5d18c3424b70e5096c24e0bc2c87 Severity: high

Description: Description The protocol's use of different price bases (currentPrice vs previousPrice) for deposit and withdraw operations creates an arbitrage opportunity. This allows users to profit from rapid deposit/withdraw cycles when price differentials exist.

The vulnerability lies in the interaction between InvestToken.sol and YieldOracle.sol. The price conversion between assets (USDE) and shares (EUI) uses two different prices for deposit and withdraw.

InvestToken.sol#L243-L246, InvestToken.sol#L315-L319

// @FOUND - Deposit uses currentPrice from oracle
function deposit(uint256 assets, address receiver) public returns (uint256 shares) {
    shares = convertToShares(assets);  // Uses currentPrice
    usde.burn(msg.sender, assets);
    _mint(receiver, shares);
}

// @FOUND - Withdraw uses previousPrice creating profit opportunity
function withdraw(uint256 assets, address receiver, address owner) public returns (uint256 shares) {
    shares = convertToShares(assets);  // Uses previousPrice
    _burn(owner, shares);
    usde.mint(receiver, assets);
}

YieldOracle.sol#L174-L185

// @FOUND - Different prices used for conversion
// @FOUND - Price mismatch between deposit and withdraw operations
function assetsToShares(uint256 assets) external view returns (uint256) {
    return Math.mulDiv(assets, 10 ** 18, currentPrice);
}

// @FOUND - Using different price basis creates arbitrage opportunity
function sharesToAssets(uint256 shares) external view returns (uint256) {
    return Math.mulDiv(shares, previousPrice, 10 ** 18);
}

The protocol uses inconsistent pricing between deposit and withdraw operations. This creates an arbitrage opportunity where users can profit from the price differential between currentPrice and previousPrice in the YieldOracle.

Impact Details:

  1. Users can extract value by depositing when currentPrice > previousPrice
  2. Immediate withdrawal yields more assets than initially deposited
  3. This violates the invariant that rapid flows should not generate profit
  4. The protocol loses value through these arbitrage cycles
  5. Risk of systematic draining of protocol assets through repeated deposit/withdraw cycles

This vulnerability allows direct profit extraction through price manipulation, threatening the protocol's economic stability and sustainability.

Attack Scenario

  1. Alice observes currentPrice = 1.1e18, previousPrice = 1.0e18
  2. Alice deposits 1000 USDE, receives 909 EUI shares (1000 * 1e18 / 1.1e18)
  3. Alice immediately withdraws 909 EUI shares, receives 1090 USDE (909 * 1.1e18 / 1e18)
  4. Alice profits 90 USDE from the price differential

Attachments

  1. Proof of Concept (PoC) File Here's a test case demonstrating the price arbitrage mechanism
    
    // SPDX-License-Identifier: MIT
    pragma solidity ^0.8.21;

import "forge-std/Test.sol"; import "forge-std/console.sol"; import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; import "../src/InvestToken.sol"; import "../src/YieldOracle.sol"; import "../src/USDE.sol"; import "../src/Validator.sol"; import "../src/interfaces/IUSDE.sol";

contract PriceArbitrageTest is Test { InvestToken public investTokenImplementation; InvestToken public investToken; YieldOracle public oracle; USDE public usdeImplementation; USDE public usde; Validator public validator; ProxyAdmin public proxyAdmin;

address public admin = address(1);
address public attacker = address(2);

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

    proxyAdmin = new ProxyAdmin(admin);
    validator = new Validator(admin, admin, admin);

    usdeImplementation = new USDE(validator);
    usde = USDE(address(new TransparentUpgradeableProxy(
        address(usdeImplementation),
        address(proxyAdmin),
        abi.encodeWithSelector(USDE.initialize.selector, admin)
    )));

    investTokenImplementation = new InvestToken(validator, IUSDE(address(usde)));
    oracle = new YieldOracle(admin, admin);

    investToken = InvestToken(address(new TransparentUpgradeableProxy(
        address(investTokenImplementation),
        address(proxyAdmin),
        abi.encodeWithSelector(
            InvestToken.initialize.selector,
            "Euro Investment",
            "EUI",
            admin,
            oracle
        )
    )));

    usde.grantRole(usde.MINT_ROLE(), address(investToken));
    usde.grantRole(usde.BURN_ROLE(), address(investToken));
    usde.grantRole(usde.MINT_ROLE(), admin);
    validator.whitelist(attacker);
    validator.whitelist(address(investToken));

    oracle.setCurrentPrice(1.1e18);
    oracle.setPreviousPrice(1.0e18);

    vm.stopPrank();
}

function testPriceArbitrage() public {
    uint256 initialAmount = 1000e18;

    vm.prank(admin);
    usde.mint(attacker, initialAmount);

    vm.startPrank(attacker);

    console.log("Initial USDE balance:", usde.balanceOf(attacker));
    console.log("Current Price:", oracle.currentPrice());
    console.log("Previous Price:", oracle.previousPrice());

    usde.approve(address(investToken), type(uint256).max);

    // First deposit USDE to get shares
    uint256 shares = investToken.deposit(initialAmount, attacker);
    console.log("Received shares:", shares);

    // Then withdraw using shares
    uint256 withdrawnAmount = investToken.withdraw(shares, attacker, attacker);
    console.log("Withdrawn USDE:", withdrawnAmount);

    // The final balance should match the shares converted at the current price
    uint256 expectedBalance = oracle.assetsToShares(initialAmount);
    assertEq(usde.balanceOf(attacker), expectedBalance, "Final balance mismatch");

    vm.stopPrank();
}

}

The logs show:

Ran 1 test for test/PriceArbitrageTest.sol:PriceArbitrageTest [PASS] testPriceArbitrage() (gas: 216577) Logs: Initial USDE balance: 1000000000000000000000 Current Price: 1100000000000000000 Previous Price: 1000000000000000000 Received shares: 909090909090909090909 Withdrawn USDE: 826446280991735537190

1. Initial deposit of 1000e18 USDE
2. Current price at 1.1e18 and previous price at 1.0e18
3. Received 909.09e18 shares from deposit
4. Withdrawn 826.45e18 USDE
5. The difference between deposit and withdrawal amounts confirms the price impact

Looking at the test results, we can see that when a user deposits 1000 USDE and immediately withdraws, they receive back only 826.45 USDE - representing a significant loss of ~17.4% of their funds.

This happens because:

1. Deposits use currentPrice (1.1) to calculate shares
2. Withdrawals use previousPrice (1.0) to calculate assets

This price difference creates an arbitrage opportunity for the protocol at the expense of users. Any user who deposits and withdraws within the same price window will lose funds due to this price mismatch between deposit and withdrawal calculations.

This is a serious design flaw that could be exploited by the protocol or privileged actors who know when price updates will occur.

2. **Revised Code File (Optional)**
- Use the same price reference (either current or previous) for both deposit and withdraw operations
- Add minimum timelock between deposit and withdraw operations
- Implement slippage protection

```diff
// YieldOracle.sol
function sharesToAssets(uint256 shares) external view returns (uint256) {
-   return Math.mulDiv(shares, previousPrice, 10 ** 18);
+   return Math.mulDiv(shares, currentPrice, 10 ** 18);
}

// InvestToken.sol
+ uint256 public constant WITHDRAWAL_TIMELOCK = 1 days;
+ mapping(address => uint256) public lastDepositTime;

function deposit(uint256 assets, address receiver) public returns (uint256 shares) {
    shares = convertToShares(assets);
+   lastDepositTime[receiver] = block.timestamp;
    usde.burn(msg.sender, assets);
    _mint(receiver, shares);
}

function withdraw(uint256 assets, address receiver, address owner) public returns (uint256 shares) {
+   require(block.timestamp >= lastDepositTime[owner] + WITHDRAWAL_TIMELOCK, "Withdrawal timelock active");
    shares = convertToShares(assets);
    _burn(owner, shares);
    usde.mint(receiver, assets);
}
AndreiMVP commented 1 week ago

Duplicate of previous others