sherlock-audit / 2024-08-woofi-solana-deployment-judging

0 stars 0 forks source link

Uneven Tin Mongoose - The implementation of `get_price_impl` is incorrect. #28

Open sherlock-admin3 opened 15 hours ago

sherlock-admin3 commented 15 hours ago

Uneven Tin Mongoose

High

The implementation of get_price_impl is incorrect.

Summary

The logic for retrieving the price is incorrect, resulting in fetching the wrong price.

Vulnerability Detail

Here is the code logic for retrieving the price from the oracle:

when woFeasible && priceWithinBound     -> woPrice, feasible
when woFeasible && !priceWithinBound    -> woPrice, infeasible
when !woFeasible && clo_preferred       -> cloPrice, feasible
when !woFeasible && !clo_preferred      -> cloPrice, infeasible

We can see that the EVM implementation is correct. Reference link. However, when we look at the get_price_impl logic:

let wo_feasible = clo_price != 0 && now <= (wo_timestamp + oracle.stale_duration);

This implementation is incorrect. The clo_price is fetched from Pyth, and the value of wo_feasible should represent the state stored in the oracle account. As a result, the boolean wo_feasible is incorrect, leading to the protocol fetching the wrong price. This is the correct implementation of isWoFeasible in the EVM:

    function isWoFeasible(address base) external view override returns (bool) {
        return infos[base].price != 0 && block.timestamp <= (timestamp + staleDuration);
    }

Additionally, the logic here is also incorrect:

if wo_feasible && wo_price_in_bound {
    price_out = wo_price;
    feasible_out = true;
} else {
    price_out = 0;
    feasible_out = false;
}

If wo_feasible is true and wo_price_in_bound is false, price_out should be wo_price, not 0.

Impact

The logic for retrieving the price is incorrect, resulting in fetching the wrong price.

Code Snippet

https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/woofi/src/instructions/get_price.rs#L45-L105

Tool used

Manual Review

Recommendation

It is recommended to refactor this logic according to the EVM implementation.

toprince commented 13 hours ago

same with https://github.com/sherlock-audit/2024-08-woofi-solana-deployment-judging/issues/9