code-423n4 / 2022-04-phuture-findings

0 stars 0 forks source link

refreshedAssetPerBaseInUQ may returns incorrect value #4

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/UniswapV2PathPriceOracle.sol#L32

Vulnerability details

Impact

Inside the cycle everytime (_asset != asset) unnecessary calculations happening and return value is updated. In a case when not a single asset equals provided _asset, return value will be equal to last asset in path instead of default value.

Proof of Concept

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/UniswapV2PathPriceOracle.sol#L32

Tools Used

Recommended Mitigation Steps

Recommending to use the following code:

function refreshedAssetPerBaseInUQ(address _asset) external override returns (uint currentAssetPerBaseInUQ) {
    currentAssetPerBaseInUQ = FixedPoint112.Q112; // default value
    for (uint i = 0; i < path.length - 1; i++) {
        address asset = path[i + 1];
        if (_asset == asset) {
            currentAssetPerBaseInUQ = currentAssetPerBaseInUQ.mulDiv(
                IUniswapV2PriceOracle(oracles[i]).refreshedAssetPerBaseInUQ(asset),
                FixedPoint112.Q112
            );
            break;
        }
    }
}
jn-lp commented 2 years ago

Here we accumulate currentAssetPerBaseInUQ on each iteration of loop until _asset is met