Trendy Brick Stallion - Precision loss in `wooracle.rs`

[sherlock-admin2]

Trendy Brick Stallion

Medium

Precision loss in wooracle.rs

Summary

In update_spread_for_new_price and update_spread_for_new_price_and_spread during the calculation of the anti_spread value division is performed before multiplication which leads to precision loss because of truncation

Root Cause

dividing before multiplication in line 163 and line 196

Internal pre-conditions

no

External pre-conditions

no

Attack Path

call update_spread_for_new_price_and_spread and update_spread_for_new_price

Impact

Loss of precision due to truncation

PoC

looking at the update_spread_for_new_price code below

pub fn update_spread_for_new_price(&mut self, price: u128) -> Result<()> {
        let pre_s = self.spread;
        let pre_p = self.price;
        if pre_p == 0 || price == 0 || pre_s >= ONE_E18_U64 {
            // previous price or current price is 0, no action is needed
            return Ok(());
        }

        let max_p = max(price, pre_p);
        let min_p = min(price, pre_p);
        // let anti_spread = (ONE_E18_U128 * ONE_E18_U128 * min_price) / max_price / (ONE_E18_U128 - pre_spread as u128);
        let calc_a = checked_mul_div(ONE_E18_U128, min_p, max_p)?;
        //@audit division before multiplication and does not follow the formula format 
        let anti_s = checked_mul_div(
            ONE_E18_U128,
            calc_a,
            ONE_E18_U128.checked_sub(pre_s as u128).unwrap(),
        )?;
        if anti_s < ONE_E18_U128 {
            let new_s = ONE_E18_U128.checked_sub(anti_s).unwrap() as u64;
            if new_s > pre_s {
                self.update_spread(new_s)?;
            }
        }
        //note: when spread is gt ONE_E18_U128 spread is not updated |

        Ok(())
    }

we can see the anti_spread comment which previews the formula used in calculating anti_spread. Again, we should be able to observe that in the formula all multiplications are performed before divisions, however when we come to the implementation of anti_spread i.e anti_s, the formula is modularized for easy implementation but this brought an issue that is in the first module calc_a division performed before it is multiplied in the anti_s which makes it lose precision.

this same case applies to update_spread_for_new_price_and_spread too

Mitigation

multiplication before division to prevent truncation of values

[toprince]

Not valid

1. 1e18 is very large for solana, most solana coin's decimal is under 9.
2. rust do not have uint256, so it will exceed u128 if do continus multiple.