Steadefi

Summary

Check for bad chainlink response is not effective allowing negative values to be accepted from price feeds

Vulnerability Details

The protocol accepts price feeds from chainlink. There are many ways these feeds can go wrong and the protocol tries to mitigate the risk by adding checks such as the one below.

If any of the above conditions are true then the function will return true indicating that the chainlink response is bad/ broken. The problem arises when checking for non-positive prices which you can see below.

The check only checks if response.answer is equal to 0. The condition does not check any prices that are less than 0, so these prices can be fed to the protocol when they should not.

Now you may be thinking that this issue may be corrected by the check for a bad price deviation which exists in the protocol, but this is not the case let me explain.

The above snippet calculates _deviation
Let us assume a price feed return value of currentResponse.answer = -1 and a previousResponse.answer = 1e18
in this case the condition above is not met so we go to the else which is

let us plug in our numbers (1e18 - (-1)) * SAFE_MULTIPLIER / uint256(prevResponse.answer);
since we are subtracting a negative from a positive number, then we are essentially just adding by 1. This will bypass the check for badPriceDeviation and allow the negative number to be used

Impact

Because _badChainlinkResponse is called by the function consult, consult is then called by consultIn18Decimals, this is called by convertToUsdValue which is finally called in calcMinTokensSlippageAmt this makes this a big issue. A negative price feed will cause big problems in accounting and essentially cause calcMinTokensSlippageAmt to be incorrect, This will miscalculate the amount of slippage allowed and therefore cause a direct loss of users and the protocols funds.

Tools Used

Manual review

Recommendations

change this

to this