Unchecked return value when adding new claims can cause fun

[hats-bug-reporter]

Github username: @Audinarey
Twitter username: audinarey
Submission hash (on-chain): 0xa44b30387ddf9d23a4220c308eb0d96433ecc5b62da6700aeb7f303a95c94b4e
Severity: low

Description:
Description
The LM_PC_Bounties_v1::addClaim(...) and LM_PC_Bounties_v1::updateClaimContributors(...) functions are used to add new claims and update existing claims respectively. The contributorAddressToClaimIds which maps each contributor's address to the claimId is an EnumerableSet whose add(...)and remove(...) functions each return boolean values, but the value is not checked for each contibutor to ensure the claim ID is added as shown on L376 and as such the function will not revert if a claimId is not successfully added/removed for a given contributor address.

File: EnumerableSet.sol
317:     function add(UintSet storage set, uint256 value) internal returns (bool) {
318:         return _add(set._inner, bytes32(value));
319:     }


327:     function remove(UintSet storage set, uint256 value) internal returns (bool) {
328:         return _remove(set._inner, bytes32(value));
329:     }



File: LM_PC_Bounties_v1.sol
349:     function addClaim(
350:         uint bountyId,
351:         Contributor[] calldata contributors,
352:         bytes calldata details
353:     )
354:         external
355:         onlyModuleRole(CLAIMANT_ROLE)
356:         validBountyId(bountyId)
357:         notLocked(bountyId)
358:         returns (uint id)
359:     {
SNIP
...
371: 
372:         uint length = contributors.length;
373:         for (uint i; i < length;) {
374:             c.contributors.push(contributors[i]);
375:             // add ClaimId to each contributor address accordingly
376:  @>         contributorAddressToClaimIds[contributors[i].addr].add(claimId);
377:             unchecked {
378:                 ++i;
379:             }
380:         }
381: 
SNIP
...
385: 
386:         return claimId;
387:     }
 

390:     function updateClaimContributors(
391:         uint claimId,
392:         Contributor[] calldata contributors
393:     )
394:         external
395:         validClaimId(claimId)
396:         notClaimed(claimId)
397:         notLocked(_claimRegistry[claimId].bountyId)
398:         onlyModuleRole(CLAIMANT_ROLE)
399:     {
SNIP
...
417: 
418:         for (uint i; i < length;) {
419:             c.contributors.push(contributors[i]);
420:             // add ClaimId again to each contributor address
421:  @>         contributorAddressToClaimIds[contributors[i].addr].add(claimId);
422:             unchecked {
423:                 ++i;
424:             }
425:         }
426: 
SNIP
...
428:     }

Attack Scenario
Describe how the vulnerability can be exploited.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

Modify the LM_PC_Bounties_v1::addClaim(...) function as shown below and also do likwise for the LM_PC_Bounties_v1::updateClaimContributors(...)function

File: LM_PC_Bounties_v1.sol
349:     function addClaim(
350:         uint bountyId,
351:         Contributor[] calldata contributors,
352:         bytes calldata details
353:     )
354:         external
355:         onlyModuleRole(CLAIMANT_ROLE)
356:         validBountyId(bountyId)
357:         notLocked(bountyId)
358:         returns (uint id)
359:     {
SNIP
...
371: 
372:         uint length = contributors.length;
373:         for (uint i; i < length;) {
374:             c.contributors.push(contributors[i]);
375:             // add ClaimId to each contributor address accordingly
376:   -       contributorAddressToClaimIds[contributors[i].addr].add(claimId);
376:   +       require(contributorAddressToClaimIds[contributors[i].addr].add(claimId), "Error mapping address to claims");
377:             unchecked {
378:                 ++i;
379:             }
380:         }
SNIP
...
385: 
386:         return claimId;
387:     }

[FHieser]

Duplicate of #124