hats-finance / Euro-Dollar-0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd

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

Lack of access control on `commitPrice` function allows unauthorized price commitments in `YieldOracle` contract #37

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

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

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

Description: Description\ The vulnerability exists in the price commitment and redemption process within the YieldOracle and InvestToken contracts. Specifically, the commitPrice function updates the previousPrice to the current currentPrice, and this updated previousPrice is used immediately in the sharesToAssets conversion function for redeeming assets. This allows a user to manipulate the value of previousPrice to redeem more assets than they should, exploiting any increase in price without an adequate delay. Anyone can call commitPrice function.

Attack Scenario\

  1. A malicious user monitors when the updatePrice function is called to update the prices and he can call commitPrice before redeem.
  2. Immediately after the price commit, they trigger a redemption, using the now-inflated previousPrice to convert their shares into a higher amount of assets than they are actually entitled to.
  3. By continuously exploiting this timing, the attacker can redeem assets at artificially high values, draining the contract of its funds.

Attachments

  1. Proof of Concept (PoC) File

https://github.com/hats-finance/Euro-Dollar-0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd/blob/c04ebafc3c6c48d612eb8df38ebd3e5b2ffa73a6/src/YieldOracle.sol#L91-L102

nextPrice - currentPrice >= 0

https://github.com/hats-finance/Euro-Dollar-0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd/blob/c04ebafc3c6c48d612eb8df38ebd3e5b2ffa73a6/src/YieldOracle.sol#L183-L185

Result can be greater than before.

https://github.com/hats-finance/Euro-Dollar-0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd/blob/c04ebafc3c6c48d612eb8df38ebd3e5b2ffa73a6/src/InvestToken.sol#L353-L360

  1. Revised Code File (Optional)
 function commitPrice() public override onlyOwner {
        require(nextPrice - currentPrice >= 0, "Price out of bounds");

        // Enforce at least commitDelay after the last update
        require(lastUpdate + commitDelay < block.timestamp, "Insufficient commit delay");

        previousPrice = currentPrice;
        currentPrice = nextPrice;
        nextPrice = NO_PRICE;

        lastPriceCommitTime = block.timestamp; // Update time of last commit

        emit PriceCommitted(currentPrice);
    }
AndreiMVP commented 3 weeks ago

In current design, it is intended behavior for commitPrice to be unrestricted. That might even incentivizes the user to pay the gas fees to commit the price without waiting for authorized role to do that. So no issue.