Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All the Funds in the contract can be drained by the protocolFeeRecipient after the Quest is Ended #456

Closed
code423n4 opened this issue Jan 30, 2023 · 4 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-605 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L100-L104

Vulnerability details

Impact

 In the ERC20Quest Contract, the withdrawFee() function can be called after the quest is ended and if the contract has funds that are not claimed 
by the claimers, by that time this function can be called and claim all the remaining funds in the contract.

Proof of Concept

  For example, suppose there are 101 tokens in the contract so 100 tokens are been allocated to claimers and 1 token to the protocolFeeRecipient 
  and after the Quest Ends, they are 50 tokens unclaimed by claimers and 1 token for protocolFeeRecipient, and now the withdrawFee() function 
  can be called a few times and all 51 tokens can be claimed by the protocolFeeRecipient.

Tools Used

 Pen and paper

Recommended Mitigation Steps

 Have a check that the function should only be called once basically like having a variable like bool  protocolFeeRecipientClaimedFees =  false 
 initially and once the  withdrawFee() function is called after the quest then make it protocolFeeRecipientClaimedFees =  true and basically if it 
 called again revert it because the Fee is already claimed.
@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Jan 30, 2023
code423n4 added a commit that referenced this issue Jan 30, 2023
@c4-judge
Copy link
Contributor

c4-judge commented Feb 6, 2023

kirk-baird marked the issue as duplicate of #23

@c4-judge
Copy link
Contributor

c4-judge commented Feb 6, 2023

kirk-baird marked the issue as partial-50

@c4-judge c4-judge added the partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) label Feb 6, 2023
@kirk-baird
Copy link

Partial credit due to low quality of write-up

@c4-judge c4-judge added duplicate-587 satisfactory satisfies C4 submission criteria; eligible for awards and removed duplicate-23 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) labels Feb 14, 2023
@c4-judge
Copy link
Contributor

kirk-baird marked the issue as satisfactory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-605 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants