No success required for Oracle market rate queries

[code423n4]

Lines of code

https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPairWithOracle.sol#L287

Vulnerability details

Impact

The system can use stale or even plainly incorrect (due to any technical malfunction) price for decision making.

For example, a malicious lender can setup a bot that tracks incorrect readings (i.e. track the state of the Oracle used and act on observing (false, wrong_price) reply whenever wrong_price is lucrative enough), and end up seizing the collateral with removeCollateral(), while the position is healthy.

Setting the severity to high as that's a clear violation of the logic of the system and asset loss for the borrower.

Proof of Concept

INFTOracle have success state and rate returned in get():

https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/interfaces/INFTOracle.sol#L8

However, the state is ignored in NFTPairWithOracle both usages, the call is (, uint256 rate) = loanParams.oracle.get(address(this), tokenId):

In removeCollateral:

https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPairWithOracle.sol#L287

And in _lend:

https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPairWithOracle.sol#L321

As lend can be in borrower favor only, while is called by a lender, there looks to be no malicious usage.

However, removeCollateral is called by a lender and there Oracle malfunction is in her favor, so it is economically viable to her to act on wrong Oracle reading that is passed to the system as a result of ignoring Oracle state.

Notice that it is not Oracle malfunction, but improper Oracle usage situation.

Recommended Mitigation Steps

Consider requiring get() success in order to use the Oracle reading:

(bool res, uint256 rate) = loanParams.oracle.get(address(this), tokenId);
require(res, "Non-valid price");

https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPairWithOracle.sol#L287

[0xm3rlin]

https://github.com/code-423n4/2022-04-abranft-findings/issues/21

[0xean]

dupe of #21