hats-finance / Euro-Dollar-0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd

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

Incorrect Price Used in Share to Asset Conversion Leading to Value Miscalculation #19

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

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

Github username: -- Twitter username: 0xgreed_ Submission hash (on-chain): 0x060c934cf1ef44031d519f4bd806088f4d53cb6d72ca7bcc3a8387052ff0880a Severity: high

Description:

Relevant Context

The YieldOracle contract maintains three price values:

These prices are used to convert between shares and assets in the protocol.

Finding Description

The sharesToAssets() function incorrectly uses previousPrice instead of currentPrice when converting shares to assets. This means the conversion is using an outdated price that does not reflect the current value of shares.

The root cause is in the sharesToAssets() implementation:

function sharesToAssets(uint256 shares) external view returns (uint256) {
    return Math.mulDiv(shares, previousPrice, 10 ** 18);
}

While the corresponding assetsToShares() function correctly uses currentPrice:

function assetsToShares(uint256 assets) external view returns (uint256) {
    return Math.mulDiv(assets, 10 ** 18, currentPrice);
}

Impact Explanation

High. This discrepancy between the actual share value and the calculated asset value could lead to:

  1. Users receiving incorrect amounts when redeeming shares
  2. Incorrect accounting in protocols relying on this oracle
  3. Financial losses due to arbitrage opportunities created by the price mismatch

Likelihood Explanation

High. This issue will affect every share to asset conversion operation, making it a consistently exploitable problem rather than an edge case.

Proof of Concept

  1. Oracle updates price from 1.0 to 1.1 (10% increase)
  2. User calls sharesToAssets() to calculate redemption value
  3. Function uses old price (1.0) instead of current price (1.1)
  4. User receives ~9.09% less assets than they should (1/1.1 = 0.909)

Recommendation

Update the sharesToAssets() function to use currentPrice instead of previousPrice:

function sharesToAssets(uint256 shares) external view returns (uint256) {
    return Math.mulDiv(shares, currentPrice, 10 ** 18);
}
AndreiMVP commented 3 weeks ago

Intended behavior. Selling of shares is done at previous price in order to prevent stealing of new yield every time price is updated. Thus user has to wait > update delay to collect yield, allowing also for the offchain update of the backing RWA assets