submitClaimRequests is deleted in approveSubmitClaimRequest function before submitClaim is called and submitClaim can revert

[hats-bug-reporter[bot]]

Github username: @ololade97 Submission hash (on-chain): 0xcf7c99704584dad86c967a69c95ec1e90a4e2aa502d0a03f2f2d501634f238bd Severity: high

Description: Description\ In the approveSubmitClaimRequest function, the submitClaimRequests mapping is deleted before calling _vault.submitClaim:

delete submitClaimRequests[_internalClaimId];

bytes32 claimId = _vault.submitClaim( // ... );

The problem is that _vault.submitClaim could potentially revert. In the submitClaim function in the HATClaimsManager contract, there are instances that could cause submitClaim function to revert.

If submitClaim reverts, delete submitClaimRequests[_internalClaimId] will not revert.

Attack Scenario\ If submitClaim reverts, delete submitClaimRequests[_internalClaimId] will not revert.

Attachments https://github.com/hats-finance/hats-contracts/blob/0d6ebbde912bc272d9b310140d434ee2aacd36d3/contracts/HATArbitrator.sol#L431C4-L474C1

https://github.com/hats-finance/hats-contracts/blob/0d6ebbde912bc272d9b310140d434ee2aacd36d3/contracts/HATClaimsManager.sol#L153

1. Proof of Concept (PoC) File

   function submitClaim(address _beneficiary, uint16 _bountyPercentage, string calldata _descriptionHash) external noActiveClaim notEmergencyPaused returns (bytes32 claimId) { address arbitratorAddress = getArbitrator(); if (!arbitratorCanSubmitClaims || arbitratorAddress != msg.sender) if (committee != msg.sender) revert OnlyCommittee(); IHATVaultsRegistry _registry = registry; uint256 withdrawPeriod = _registry.getWithdrawPeriod(); // require we are in safetyPeriod // solhint-disable-next-line not-rely-on-time if (block.timestamp % (withdrawPeriod + _registry.getSafetyPeriod()) < withdrawPeriod) revert NotSafetyPeriod(); if (_bountyPercentage > maxBounty) revert BountyPercentageHigherThanMaxBounty();

   if (maxBounty == HUNDRED_PERCENT && _bountyPercentage != HUNDRED_PERCENT && _bountyPercentage > MAX_BOUNTY_LIMIT)
       revert PayoutMustBeUpToMaxBountyLimitOrHundredPercent();

2. Revised Code File (Optional)

[bahurum]

I quote your report:

If submitClaim reverts, delete submitClaimRequests[_internalClaimId] will not revert.

If submitClaim() reverts, the whole call to approveSubmitClaimRequest(), reverts. This reverts all state changes made during the call, including delete submitClaimRequests[_internalClaimId]. So there is no problem related to submitClaim() reverting.