Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

jkoppel - Post-contest refunds block tier winners in tiered fixed bounty from claiming funds #103

Closed
github-actions bot opened this issue Feb 21, 2023 · 4 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Escalation Resolved This issue's escalations have been approved/rejected High A valid High severity issue Reward A payout will be made for this issue

Comments

@github-actions
Copy link

github-actions bot commented Feb 21, 2023

jkoppel

high

Post-contest refunds block tier winners in tiered fixed bounty from claiming funds

Summary

Withdrawing deposits after the close of a tiered fixed bounty competition makes it impossible for some winners to claim funds.

Vulnerability Detail

There is a tiered fixed bounty where first place gets 9000 tokens, second place gets 1000 tokens.

The issuer funds the bounty for 5000, asks the community to fund the rest. A griefer pretends to be a "benefactor" and funds the rest. No-one else contributes because the bounty is fully funded.

The contest ends. Second place claim his 1000 tokens.

First place tries to claim his 9000 tokens, but the griefer front-runs him and withdraws his 5000 tokens. There are now only 4000 tokens left in the account, and it is not possible to transfer 9000 tokens to the winner. Nor is it possible to deposit more funds because the bounty is closed.

This can also happen if a well-intentioned person funds the contest, but then backs out.

It is still possible for the other depositors to refund their deposits. However, there is a very difficult to attack that does this while also making it possible to do refunds. In this attack, a variant of https://github.com/sherlock-audit/2023-02-openq-jkoppel/issues/3 , the griefer also spams a very large number of expired 1-token deposits so that BountyCore.getLockedFunds will require an amount of gas of almost, but not quite, the block gas limit. It is then still possible for the attacker to withdraw his 1 token and carry out the attack above. However, the attacker then calls extendDeposit on many deposits, causing the if-branch in getLockedFunds to be entered, pushing the cost of getLockedFunds over the limit, and making it possible to do any more refunds. By doing this, the attacker can cause the remaining funds in the bounty to be lost for an arbitrarily long amount of time, say 1 billion years. Repeated deposit extensions can make the amount of time the funds are frozen truly infinite.

Impact

This makes it possible for anyone to, at low difficulty, block the last claimant from a tiered percentage bounty contest from claiming. It is possible for this to occur accidentally even if all parties are well-intentioned.

With much higher difficulty, it becomes possible to freeze funds in the bounty forever.

Code Snippet

Fromhttps://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/TieredFixedBountyV1.sol#L103

        uint256 claimedBalance = payoutSchedule[_tier];

This will fail if there is less than the claimed balance left in the contract, which will occur if enough refunds have been done to make the contract no longer fully funded.

Tool used

Manual Review

Recommendation

Make it so that, if not enough funds are left in the account, claimant gets whatever is left.

And perhaps also block refunds for a period after the contest ends.

Duplicate of #77

@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue labels Feb 21, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Mar 7, 2023
@jkoppel
Copy link

jkoppel commented Mar 7, 2023

Escalate for 52 USDC

This is not a duplicate of #77. It is similar to #266, but not a duplicate, as that only reports the issue in tiered percentage bounties, while this for for tiered fixed bounties.

The nastiest version of this attack requires combining it with another attack that disables refunds. The one I suggested in the initial report is related to #77, but subtler. However, it can also be combined with other attacks for preventing refunds, including #267 or #263. That very much does not make it a duplicate of #77.

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 7, 2023

Escalate for 52 USDC

This is not a duplicate of #77. It is similar to #266, but not a duplicate, as that only reports the issue in tiered percentage bounties, while this for for tiered fixed bounties.

The nastiest version of this attack requires combining it with another attack that disables refunds. The one I suggested in the initial report is related to #77, but subtler. However, it can also be combined with other attacks for preventing refunds, including #267 or #263. That very much does not make it a duplicate of #77.

You've created a valid escalation for 52 USDC!

To remove the escalation from consideration: Delete your comment.
To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

@sherlock-admin sherlock-admin added the Escalated This issue contains a pending escalation label Mar 7, 2023
@hrishibhat
Copy link
Contributor

Escalation accepted

Not a valid duplicate of #77. Duplicate of #266 as it is the same issue occurring in two contracts.

@sherlock-admin
Copy link
Contributor

Escalation accepted

Not a valid duplicate of #77. Duplicate of #266 as it is the same issue occurring in two contracts.

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.

@sherlock-admin sherlock-admin added Escalation Resolved This issue's escalations have been approved/rejected and removed Escalated This issue contains a pending escalation labels Mar 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Escalation Resolved This issue's escalations have been approved/rejected High A valid High severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

3 participants