sherlock-audit / 2023-11-olympus-judging

9 stars 7 forks source link

tvdung94 - Balances and scaling factors length should be checked before scaling #139

Closed sherlock-admin closed 6 months ago

sherlock-admin commented 6 months ago

tvdung94

medium

Balances and scaling factors length should be checked before scaling

Summary

Balances and scaling factors length should be checked before scaling

Vulnerability Detail

In getTokenPriceFromStablePool(), before scaling balance for stablemath usage, we should check if balances' and scaling factors' lengths are equal. Different length, especially when balances has longer length than scaling factor, might lead to incorrect calculations.

 >>>(, uint256[] memory balances_, ) = balVault.getPoolTokens(poolId);
                try pool.getLastInvariant() returns (uint256, uint256 ampFactor) {
                    // Upscale balances by the scaling factors
                    uint256[] memory scalingFactors = pool.getScalingFactors();

                   >>> uint256 len = scalingFactors.length;  // @audit - should check if balance length and scaling length are equal
                    for (uint256 i; i < len; ++i) {
                        balances_[i] = FixedPoint.mulDown(balances_[i], scalingFactors[i]);
                    }

Impact

Different length between balances and scaling factors might lead to revert or incorrect price calculation.

Code Snippet

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

Tool used

Manual Review

Recommendation

Consider adding the length check

```solidity
 (, uint256[] memory balances_, ) = balVault.getPoolTokens(poolId);
                try pool.getLastInvariant() returns (uint256, uint256 ampFactor) {
                    // Upscale balances by the scaling factors
                    uint256[] memory scalingFactors = pool.getScalingFactors();

                    uint256 len = scalingFactors.length;  // @audit - should check if balance length and scaling length are equal
          >>>   if (balances.length != len) revert();
                    for (uint256 i; i < len; ++i) {
                        balances_[i] = FixedPoint.mulDown(balances_[i], scalingFactors[i]);
                    }
nevillehuang commented 6 months ago

Invalid, as seen here, there is an assigned scaling factors for each token in a balancer pool, so length of balances_ and scalingFactors will always be equal.