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

1 stars 2 forks source link

withdrawRemainingTokens in Erc20Quest could be called several times by the owner, allowing him to withdraw part of the non-claimable tokens #638

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/main/contracts/Erc20Quest.sol#L81-L87

Vulnerability details

Impact

This would allow the owner to steal the ERC20 rewards which have not yet been claimed after the end of the claim, contradicting the natspec of withdrawRemainingTokens

Proof of Concept

The owner simply has to wait the end of the quest and then call multiple times the withdrawRemainingTokens function.

Recommended Mitigation Steps

Add a state variable ownerHasWithdrawnRemaining which would be set to true the first time the owner calls withdrawRemainingTokens, and add a require(!ownerHasWithdrawnRemaining) at the start of the function.

kirk-baird commented 1 year ago

This is true but I don't see the benefit to calling this multiple times

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