sherlock-audit / 2023-11-olympus-judging

9 stars 8 forks source link

cu5t0mPe0 - `BalancerPoolTokenPrice.sol` wrongly assumes that all of the weighted pools uses `totalSupply` #165

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

cu5t0mPe0

high

BalancerPoolTokenPrice.sol wrongly assumes that all of the weighted pools uses totalSupply

Summary

BalancerPoolTokenPrice.sol, specifically for weighted balancer pools, incorrectly always uses totalSupply to get the pool's total supply, but this is not the case for newer weighted pools.

Vulnerability Detail

According to Balancer's official documentation, Balancer uses getActualSupply for the latest versions of weighted pools and stable pools, while totalSupply only makes sense for the old "legacy" pool.

https://docs.balancer.fi/concepts/advanced/valuing-bpt/valuing-bpt.html#on-chain

In the contract BalancerPoolTokenPrice, the function getWeightedPoolTokenPrice calls totalSupply to obtain the total supply, which will eventually lead to an error in price calculation in the pool, which will harm the protocol and users.

Impact

The impact is very serious. The token price in the pool will be calculated incorrectly. This error may not be correctly discovered when the token in the balancer pool is manipulated.

Code Snippet

https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/PRICE/submodules/feeds/BalancerPoolTokenPrice.sol#L402-L402

Tool used

Manual Review

Recommendation

Since the protocol needs to interact with multiple Balancer pools, you should first try to check if the weighted pool you are interacting with is newer and uses getActualSupply or if it is older and uses totalSupply

Duplicate of #155