THe removeFeeder method is lacking onlyRole(*DEFAULT_ADMIN_ROLE*) modifier, even though the NatSpec states “Allows owner to remove feeder”. Due to this, now everyone can remove a feeder anytime. Different types of attack can be executed, one of which is the following:
Some price changes and now a feeder tries to call setPrice
A malicious user does not like the price change reported, so he front-runs that transaction with removeFeeder transaction
The price update fails, so now price is stale
Repeat as many times as needed
Severity
The likelihood of this happening is high because attacks on missing access control are very easy to execute. The impact is also high because an attacker can force the protocol to operate with a stale price or even to not have any price updaters apart from the owner of the protocol. Since both likelihood and impact are high this should be a High severity vulnerability.
Recommendation
Add onlyRole(DEFAULT_ADMIN_ROLE) modifier to the removeFeeder method
Lines of code
https://github.com/code-423n4/2022-11-paraspace/blob/6542d6e946762fc7a914a3f6f2e08fcbbf6c8a13/paraspace-core/contracts/misc/NFTFloorOracle.sol#L167
Vulnerability details
Proof of Concept
THe
removeFeeder
method is lackingonlyRole(*DEFAULT_ADMIN_ROLE*)
modifier, even though the NatSpec states “Allows owner to remove feeder”. Due to this, now everyone can remove a feeder anytime. Different types of attack can be executed, one of which is the following:setPrice
removeFeeder
transactionSeverity
The likelihood of this happening is high because attacks on missing access control are very easy to execute. The impact is also high because an attacker can force the protocol to operate with a stale price or even to not have any price updaters apart from the owner of the protocol. Since both likelihood and impact are high this should be a High severity vulnerability.
Recommendation
Add
onlyRole(DEFAULT_ADMIN_ROLE)
modifier to theremoveFeeder
method