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 percentage bounty from claiming funds #101

Closed
github-actions bot opened this issue Feb 21, 2023 · 6 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 percentage bounty from claiming funds

Summary

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

Vulnerability Detail

There is a tiered percentage bounty where first place gets 90%, second place gets 10%

The bounty has 10e18 tokens in it.

A griefer deposits 10 tokens. There are now 10e18 + 10 tokens in the account.

The contest ends. Second place claim his 1e18 + 1 tokens.

The first place winner tries to claim his 9e18 + 9 tokens, but the griefer front-runs him by withdrawing his 10 tokens. There are now only 9e18 tokens left in the account, and it is not possible to transfer 9e18+9 tokens out of the account.

First place winner is now unable to claim anything, as his tier is no longer sufficiently funded.

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 impossible 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 impossible 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 and virtually no cost, block the last claimant from a tiered percentage bounty contest from claiming. It is also possible to block other claimants as well, at higher cost and equally-low difficulty. With much higher difficulty, it becomes possible to freeze funds in the bounty forever.

Code Snippet

From https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/TieredPercentageBountyV1.sol#L115

        uint256 claimedBalance = (payoutSchedule[_tier] *
            fundingTotals[_tokenAddress]) / 100;

        _transferToken(_tokenAddress, claimedBalance, _payoutAddress);

This will fail if there is less than the claimed balance left in the contract, which will occur if any refunds have been done.

Note that fundingTotals is computed by closeCompetition above and is fixed from then on.

Tool used

Manual Review

Recommendation

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

Duplicate of #266

@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 43 USDC

This is not a duplicate of #77. #77 is entirely about an OOG attack. There is a note in this report about how it can be combined with an out-of-gas attack to become more severe, but the core issue here has nothing to do with gas.

It is instead a duplicate of #266.

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 7, 2023

Escalate for 43 USDC

This is not a duplicate of #77. #77 is entirely about an OOG attack. There is a note in this report about how it can be combined with an out-of-gas attack to become more severe, but the core issue here has nothing to do with gas.

It is instead a duplicate of #266.

You've created a valid escalation for 43 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
@pauliax
Copy link

pauliax commented Mar 7, 2023

Escalate for 11 USDC

This is not a duplicate of #77. #77 is entirely about an OOG attack. There is a note in this report about how it can be combined with an out-of-gas attack to become more severe, but the core issue here has nothing to do with gas.

Escalate for 11 USDC.

Agree with @jkoppel, this issue is clearly not a duplicate of #77 and is kinda related to what I wrote here so might be useful for context when re-judging: #505 (comment)

My two cents are that this issue should be Medium, not High because post-contest refunders risk that claims will come first, or if their deposits are small enough, the bounty can be made whole by sending these tokens directly to the contract. So they have to make large enough deposits and hope their transactions will be mined earlier than claimants.

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 7, 2023

Escalate for 11 USDC

This is not a duplicate of #77. #77 is entirely about an OOG attack. There is a note in this report about how it can be combined with an out-of-gas attack to become more severe, but the core issue here has nothing to do with gas.

Escalate for 11 USDC.

Agree with @jkoppel, this issue is clearly not a duplicate of #77 and is kinda related to what I wrote here so might be useful for context when re-judging: #505 (comment)

My two cents are that this issue should be Medium, not High because post-contest refunders risk that claims will come first, or if their deposits are small enough, the bounty can be made whole by sending these tokens directly to the contract. So they have to make large enough deposits and hope their transactions will be mined earlier than claimants.

You've created a valid escalation for 11 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.

@hrishibhat
Copy link
Contributor

Escalation accepted

Valid duplicate of #266

@sherlock-admin
Copy link
Contributor

Escalation accepted

Valid duplicate of #266

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 15, 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

4 participants