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

2 stars 2 forks source link

0xeix - wo_price_in_bound has incorrect check implementation #8

Open sherlock-admin4 opened 1 month ago

sherlock-admin4 commented 1 month ago

0xeix

Medium

wo_price_in_bound has incorrect check implementation

Summary

wo_price_in_bound has an incorrect condition statement that can make it return an incorrect value.

Vulnerability Detail

In the current implementation of the wo_price_in_bound, the protocol uses the following check:

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

 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);

First, we check if the clo_price != 0 but this is incorrect way to handle it and should be done like it's introduced in the EVM implementation of the contracts:

https://github.com/woonetwork/woofi_swap_smart_contracts/blob/main/contracts/WooracleV2.sol#L202-203

    bool checkWoBound = cloPrice == 0 ||
     (cloPrice.mulFloor(1e18 - bound) <= woPrice && woPrice <= cloPrice.mulCeil(1e18 + bound));

This check assures whether the derived clo_price reflects not feasible value or not and if so, this would make checkWoBound automatically set to true as we don't fetch zero-valued clo_price. But, in the current functionality, we check if the clo_price != 0 and the wo_price are in bounds which is not a proper way to do it.

Impact

The current functionality for the contract does not ensure compatibility with the EVM implementation and does not perform correct checks of the prices that will result in a derived value being different from reality.

Code Snippet

Provided above.

Tool used

Manual Review

Recommendation

Change the check to make sure clo_price != 0 and if so, make wo_price_in_bound set to true.

toprince commented 1 month ago

Not valid for this one. Please check https://github.com/woonetwork/WooPoolV2/blob/main/contracts/wooracle/WooracleV2_2.sol#L278-L283 This is the one for evm after last audit...