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

2 stars 2 forks source link

pashap9990 - get_price function returns stale price #66

Open sherlock-admin2 opened 1 month ago

sherlock-admin2 commented 1 month ago

pashap9990

High

get_price function returns stale price

Summary

every pool has a oracle and oracle's price will be updated through off-chain mechanism but this can cause stale price

We have an offchain script, posting the prices of supported tokens (sol, usdt, etc) to Wooracle contract on Solana.

Root Cause

Admin can update coeff and spread directly but when admin call set_coeff_handler and set_spread_handler directly which in turn call update_now ,hence updated_at sets current timestamp

Code Snippet

https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/woofi/src/instructions/admin/set_woo_state.rs#L91

https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/woofi/src/instructions/admin/set_woo_state.rs#L96

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

Impact

get_price function return stale price

PoC

Textual PoC: lets assume stale_duration is 1 min 1-offchain script calls set_price_handler[update_at=T1,price=p1] 2-Admin calls set_coeff_handler manually to update coeff at T1 + 30 secs[update_at=T1 + 30 secs, price=p1] 3-asset's price change from p1 to p2 4-pyth network cannot provide price for every reason

Alternatively, a network outage (at the internet level, blockchain level, or at multiple data providers) may prevent the protocol from producing new price updates. (Such outages are unlikely, but integrators should still be prepared for the possibility.) In such cases, Pyth may return a stale price for the product.

5-price is valid until T1 + 30 sec + 60 sec but transaction has to be reverted after T1 + 60 sec

pub fn get_price_impl<'info>(
    oracle: &Account<'info, Wooracle>,
    price_update: &mut Account<'info, PriceUpdateV2>,
    quote_price_update: &mut Account<'info, PriceUpdateV2>,
    ...

@>>> let wo_feasible = clo_price != 0 && now <= (wo_timestamp + oracle.stale_duration);
    let wo_price_in_bound = clo_price != 0
        && ((clo_price * (ONE_E18_U128 - bound)) / ONE_E18_U128 <= wo_price
            && wo_price <= (clo_price * (ONE_E18_U128 + bound)) / ONE_E18_U128);

Mitigation

 pub fn set_coeff_handler(ctx: Context<SetWooStateOnlyAdmin>, coeff: u64) -> Result<()> {
     ctx.accounts.wooracle.update_coeff(coeff)?;
-    ctx.accounts.wooracle.update_now()
+    
 }

 pub fn set_spread_handler(ctx: Context<SetWooStateOnlyAdmin>, spread: u64) -> Result<()> {
     ctx.accounts.wooracle.update_spread(spread)?;
-    ctx.accounts.wooracle.update_now()
+    
 }
toprince commented 1 month ago

Valid, But coeff and spread is not updated frequently, several times per month. So low impact.