clems4ever - Multiple payments for one contributor but only one invoice required

[github-actions]

clems4ever

medium

Multiple payments for one contributor but only one invoice required

Summary

In OngoingBounty, a claimId is a hash encoding the contributor and the PR and this id is used as a key for whether supporting documents have been provided or an invoice has been completed. However, if a contributor makes several contributions and get paid multiple times, I guess we should verify that there is a new invoice for each payment. However, this is not checked in the contract since the mapping are all overridden since multiple payouts would generate the same claimId.

Vulnerability Detail

function claimOngoingPayout(
        address _payoutAddress,
        bytes calldata _closerData
    ) external onlyClaimManager nonReentrant returns (address, uint256) {
        (, string memory claimant, , string memory claimantAsset) = abi.decode(
            _closerData,
            (address, string, address, string)
        );

        bytes32 _claimId = generateClaimId(claimant, claimantAsset);

        claimId[_claimId] = true; <================================================ same claim id for same user on same PR who makes 2 contributions or more and this makes overridden with the same value. Is it expected?
        claimIds.push(_claimId);

        _transferToken(payoutTokenAddress, payoutVolume, _payoutAddress);
        return (payoutTokenAddress, payoutVolume);
    }

Impact

Only one invoice required to get the payment instead of as many invoices as there are payments for the same developers on the same PR.

Code Snippet

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/OngoingBountyV1.sol#L107

Tool used

Manual Review

Recommendation

Include the number of payments in the encoding function for generating a unique claim id for every claim from the same user and PR in order to verify whether there is an invoice for each payment.

[FlacoJones]

Will fix by removing Ongoing

[FlacoJones]

OpenQDev/OpenQ-Contracts#112

[hrishibhat]

Given the _claimId being unique for a given submission the above assumption seems to be incorrect.