code-423n4 / 2022-11-paraspace-findings

7 stars 4 forks source link

Anyone can remove feeders from `NFTFloorOracle`. #428

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L167-L172

Vulnerability details

Impact

There is no onlyRole modifier in removeFeeder, so anyone can remove feeders from NFTFloorOracle, and it will cause a DOS attack.

Proof of Concept

    function removeFeeder(address _feeder)
        external
        onlyWhenFeederExisted(_feeder) //@audit no modifier
    {
        _removeFeeder(_feeder);
    }

There is onlyRole modifier in addFeeders, so only default admin can add feeders. But there is no onlyRole modifier in removeFeeder, so anyone can remove feeders from NFTFloorOracle. If the number of feeders are less than MIN_ORACLES_NUM, _combine will return false for dataValidity, and we can't set price using _finalizePrice in setPrice. So it can induce a DOS attack.

Tools Used

Manual Review

Recommended Mitigation Steps

Add onlyRole modifier in removeFeeder.

c4-judge commented 1 year ago

dmvt marked the issue as duplicate of #31

c4-judge commented 1 year ago

dmvt marked the issue as satisfactory