search

code-423n4 / 2023-01-rabbithole-findings

1 stars 2 forks source link

Missing upper bound for royaltyFee_ #667

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L90-L93

Vulnerability details

Since the setRoyaltyFee() function does not impose an upper bound for royaltyFee_ parameter, it is possible to set an extremely high value of royaltyFee_ . The royalty fee should typically have a lower value.

Proof of Concept

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L90-L93

File: contracts/RabbitHoleReceipt.sol

90-93: function setRoyaltyFee(uint256 royaltyFee_) public onlyOwner {
        royaltyFee = royaltyFee_;
        emit RoyaltyFeeSet(royaltyFee_);
    }

Recommended Mitigation Steps

Set an upper bound for royaltyFee_ similar to that implemented for setQuestFee() function. For instance,

if (royaltyFee_ > 500) revert RoyaltyFeeTooHigh();
c4-judge commented 1 year ago

kirk-baird changed the severity to QA (Quality Assurance)

c4-sponsor commented 1 year ago

waynehoover marked the issue as sponsor acknowledged

c4-judge commented 1 year ago

kirk-baird marked the issue as grade-b