Platform: Code4rena
Start Date: 28/11/2022
Pot Size: $192,500 USDC
Total HM: 33
Participants: 106
Period: 11 days
Judge: LSDan
Total Solo HM: 15
Id: 186
League: ETH
Rank: 60/106
Findings: 2
Award: $122.23
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: IllIllI
Also found by: 0x52, 0xNazgul, Franfran, IllIllI, Jeiwan, Lambda, RaymondFam, Rolezn, Trust, __141345__, codecustard, erictee, gzeon, hansfriese, imare, rbserver, rvierdiiev, seyni, skinz, ujamal_
18.3064 USDC - $18.31
ParaSpaceOracle.getAssetPrice doesn't check if price is fresh. As result stale price can be used which can lead to protocol loses if price has changed.
function getAssetPrice(address asset) public view override returns (uint256) { if (asset == BASE_CURRENCY) { return BASE_CURRENCY_UNIT; } uint256 price = 0; IEACAggregatorProxy source = IEACAggregatorProxy(assetsSources[asset]); if (address(source) != address(0)) { price = uint256(source.latestAnswer()); } if (price == 0 && address(_fallbackOracle) != address(0)) { price = _fallbackOracle.getAssetPrice(asset); } require(price != 0, Errors.ORACLE_PRICE_NOT_READY); return price; }
If IEACAggregatorProxy exists for asset then it's price is fetched using source.latestAnswer() function. After the price is fetched there is only check that price is not 0. But there is no check if the price is fresh. It's possible that price will be not 0, but it will be stale. And this can lead to protocol loses if the real price has changed meanwhile the aggregator was not working.
VsCode
IEACAggregatorProxy also has latestTimestamp() function. Result of this function should be checked with some stake period to detect if the price is fresh enough.
#0 - JeffCX
2022-12-18T03:29:24Z
#1 - c4-judge
2022-12-20T17:44:38Z
dmvt marked the issue as duplicate of #5
#2 - c4-judge
2023-01-23T15:57:19Z
dmvt marked the issue as satisfactory
🌟 Selected for report: IllIllI
Also found by: 0x4non, 0x52, 0xAgro, 0xNazgul, 0xSmartContract, 0xackermann, 9svR6w, Awesome, Aymen0909, B2, BRONZEDISC, Bnke0x0, Deekshith99, Deivitto, Diana, Dravee, HE1M, Jeiwan, Kaiziron, KingNFT, Lambda, Mukund, PaludoX0, RaymondFam, Rolezn, Sathish9098, Secureverse, SmartSek, __141345__, ahmedov, ayeslick, brgltd, cccz, ch0bu, chrisdior4, cryptonue, cryptostellar5, csanuragjain, datapunk, delfin454000, erictee, gz627, gzeon, helios, i_got_hacked, ignacio, imare, jadezti, jayphbee, joestakey, kankodu, ksk2345, ladboy233, martin, nadin, nicobevi, oyc_109, pashov, pavankv, pedr02b2, pzeus, rbserver, ronnyx2017, rvierdiiev, shark, unforgiven, xiaoming90, yjrwkk
103.9175 USDC - $103.92
It's possible to pause not existing asset in NFTFloorOracle. As a result when asset will be added it will be paused and will be not possible to set and get price for it until it will be unpaused.
function setPause(address _asset, bool _flag) external onlyRole(DEFAULT_ADMIN_ROLE) { assetFeederMap[_asset].paused = _flag; emit AssetPaused(_asset, _flag); }
NFTFloorOracle.setPause allows to pause asset and restrict setting price for it. Because there is no onlyWhenAssetExisted modifier it's possible to pause asset that doesn't exist. Later, when admin will add asset it will be paused already and it will be not possible to set price for it until it will be noticed and admin unpause asset.
VsCode
Add nlyWhenAssetExisted modifier to setPause function.
#0 - c4-judge
2023-01-09T22:33:57Z
dmvt changed the severity to QA (Quality Assurance)
#1 - c4-judge
2023-01-25T12:33:17Z
dmvt marked the issue as grade-b