ltyu - Deposits can be refunded after closing

[github-actions]

ltyu

high

Deposits can be refunded after closing

Summary

Depositors can refund their deposit at anytime causing insufficient balance for claims.

Vulnerability Detail

According to the natspec documentation and discussion with FlacoJones, closeCompetition() should “freezes the current funds for the competition.” This is not the case for depositor refunds. Depositors can refund their deposit at anytime. This is problematic because if a deposit is refunded after the competition has closed, there may not be enough tokens to fund the claims.

Impact

Code Snippet

This is the refund logic in all Bounty contracts. Notice that there are no restrictions in regards status
https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/BountyCore.sol#L64-L93

Proof of concept

it('should allow refunds after close', async () => {
    // ARRANGE
    const volume = 100;
    
    // ASSUME
    const linkDepositId = generateDepositId(Constants.bountyId, 0);
    await atomicContract.connect(depositManager).receiveFunds(owner.address, mockLink.address, volume, 1);
    expect(await atomicContract.refunded(linkDepositId)).to.equal(false);
    
    const daiDepositId = generateDepositId(Constants.bountyId, 1);
    await atomicContract.connect(depositManager).receiveFunds(owner.address, mockDai.address, volume, 1);
    expect(await atomicContract.refunded(daiDepositId)).to.equal(false);
    
    const protocolDepositId = generateDepositId(Constants.bountyId, 2);
    await atomicContract.connect(depositManager).receiveFunds(owner.address, ethers.constants.AddressZero, volume, 1, { value: volume });
    expect(await atomicContract.refunded(protocolDepositId)).to.equal(false);
    
    // Close
    await expect(await atomicContract.status()).equals(0);
    await atomicContract.connect(claimManager).close(owner.address, closerData);
    await expect(await atomicContract.status()).equals(1);
    
    // ACT
    await atomicContract.connect(depositManager).refundDeposit(linkDepositId, owner.address, volume);
    await atomicContract.connect(depositManager).refundDeposit(daiDepositId, owner.address, volume);
    await atomicContract.connect(depositManager).refundDeposit(protocolDepositId, owner.address, volume);
    
    // ASSERT
    expect(await atomicContract.refunded(linkDepositId)).to.equal(true);
    expect(await atomicContract.refunded(daiDepositId)).to.equal(true);
    expect(await atomicContract.refunded(protocolDepositId)).to.equal(true);
});

Tool used

Manual Review

Recommendation

Consider validating if the competition has closed for deposit refunds.

Duplicate of #266