Miss checks for price validity

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-polynomial/blob/aeecafc8aaceab1ebeb94117459946032ccdff1e/src/Exchange.sol#L155-L170 https://github.com/code-423n4/2023-03-polynomial/blob/aeecafc8aaceab1ebeb94117459946032ccdff1e/src/Exchange.sol#L186-L190 https://github.com/code-423n4/2023-03-polynomial/blob/aeecafc8aaceab1ebeb94117459946032ccdff1e/src/Exchange.sol#L409-L410 https://github.com/code-423n4/2023-03-polynomial/blob/aeecafc8aaceab1ebeb94117459946032ccdff1e/src/KangarooVault.sol#L436-L437 https://github.com/code-423n4/2023-03-polynomial/blob/aeecafc8aaceab1ebeb94117459946032ccdff1e/src/KangarooVault.sol#L568

Vulnerability details

Impact

The price returned from pool or market could be invalid, maybe due to stale or malfunction of oracles. However, the checks for validity of price feed is not consistent across the contracts. The outdated/stale/invalid price data could lead to inaccurate price feed for index price, mark price and funding rate, and further influence the operation of the whole protocol.

Proof of Concept

In getIndexPrice(), getFundingRate() and getMarkPrice(), isInvalid is assigned be not checked.

File: src/Exchange.sol
155:     function getIndexPrice() public view override returns (uint256 indexPrice, bool isInvalid) {
156:         (uint256 baseAssetPrice, bool invalid) = pool.baseAssetPrice();
157:         isInvalid = invalid;
158: 
159:         // base_asset_price^2 / pricing_constant
160:         indexPrice = baseAssetPrice.mulDivDown(baseAssetPrice, PRICING_CONSTANT);
161: 
162:         // multiply by normalization factor for applying historic funding rate
163:         indexPrice = indexPrice.mulWadDown(normalizationFactor);
164:     }

167:     function getFundingRate() public view override returns (int256 fundingRate, bool isInvalid) {
168:         // Index price
169:         (uint256 indexPrice, bool invalid) = getIndexPrice();
170:         isInvalid = invalid;

186:     function getMarkPrice() public view override returns (uint256 markPrice, bool isInvalid) {
187:         (uint256 baseAssetPrice, bool invalid) = pool.baseAssetPrice();
188:         isInvalid = invalid;
189: 
190:         (int256 fundingRate,) = getFundingRate();

409:     function _updateFundingRate() internal {
410:         (int256 fundingRate,) = getFundingRate();

In KangarooVault.sol, the call to getMarkPrice() miss validity checks for the following:

File: src/KangarooVault.sol
436:     function removeCollateral(uint256 collateralToRemove) external requiresAuth nonReentrant {
437:         (uint256 markPrice,) = LIQUIDITY_POOL.getMarkPrice();

556:     function _openPosition(uint256 amt, uint256 minCost) internal {

568:         (uint256 markPrice,) = LIQUIDITY_POOL.getMarkPrice();

In many other places across the contracts, the price validity is checked, but the above places just miss the checks.

Tools Used

Manual analysis.

Recommended Mitigation Steps

Add require(!isInvalid) in the above places to check the price validity. Just like other price queries in the contracts.

[c4-judge]

JustDravee marked the issue as duplicate of #59

[c4-judge]

JustDravee changed the severity to QA (Quality Assurance)

[c4-judge]

This previously downgraded issue has been upgraded by JustDravee

[c4-judge]

JustDravee marked the issue as satisfactory