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

1 stars 2 forks source link

Critical parameter endTime value has no maximum value defined #679

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L81-L87 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Quest.sol#L149-L151

Vulnerability details

I'm not sure if this issue can be considered as a Medium or Low issue: It seems the only way to withdraw the balance from a Quest contract is by calling withdrawRemainingTokens() and withdrawFee(). Both functions needs that block.timestamp > endTime to pass, but this also means that if a mistake is done and endTime is wrongly set with a very long value in the future, then the tokens will be stuck on the contract.

Impact

If a wrong endTime value is set, the tokens will be stuck inside the contract for a very long time.

Tools Used

Manual checking

Recommended Mitigation Steps

Set an upper bound for endTime by also comparing with the current timestamp

(I'm sorry if I'm not providing any POC, I'll let you decide if to accept this or not. I lost access to my previous account and I'm doing these submissions at the very last minute, I thank you for activating my new account in a very short time)

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

kirk-baird commented 1 year ago

Valid issue but insufficient quantity of issues to qualify for grade-b

c4-judge commented 1 year ago

kirk-baird marked the issue as grade-c