The requiredVouches is checked with a wrong sign

[hats-bug-reporter[bot]]

Github username: -- Twitter username: DrippyRobin Submission hash (on-chain): 0x4436a45f22503c6e00e44aebfd3ee3e11d516056f6939baca54b7238e2b1507f Severity: medium

Description: Description\ If the intention is to ensure that the loop runs until the exact number of required vouches is accumulated, then < does not accurately represent this logic. The correct check should be <= to ensure that the loop exits when request.vouches.length is equal to requiredVouches. The loop continues running as long as request.vouches.length is less than requiredVouches.

Attack Scenario\ If requiredNumberOfVouches is set to 100, the loop condition while (request.vouches.length < requiredNumberOfVouches) will continue executing as long as request.vouches.length is less than 100 even though the required number is 100, that means the while loop will pass even on 99 while required is 100

The loop will terminate once request.vouches.length is equal to requiredVouches - 1. This means that if you need exactly 100 valid vouches (requiredVouches = 100), the loop will stop after reaching only 99 valid vouches (request.vouches.length = 99). The loop won't run again to add the 100th vouch.

Attachments

1. https://github.com/hats-finance/Proof-Of-Humanity-V2-0xef0709445d394a22704850c772a28a863bb780b0/blob/07f6529954291f79b3c690881004f306da97759f/contracts/ProofOfHumanity.sol#L927

2. Proof of Concept (PoC) File The loop's exit means that the required number of vouches might never be fully accumulated. As a result, the conditions necessary for advancing the request from the "Vouching" state to the "Resolving" state are not met, so even if the user has 100 valid vouches, they wont be able to pass so it will make the function to always overflow.

3. Revised Code File (Optional) To ensure that the loop continues until the exact number of required vouches is collected, the condition should be <=:

-    while (request.vouches.length < requiredVouches
+    while (request.vouches.length <= requiredVouches) {
   // Loop logic
}

By using <=, the loop will continue to run until request.vouches.length reaches requiredVouches. So, if requiredVouches is 100, the loop will continue until 100 vouches have been collected. This guarantees that the function will meet the exact requirement before stopping.

[Robinx33]

Means the 100th Vouch in the list wont be checked

[Robinx33]

I made a mistake, this means The request might be advanced to the Resolving state even if there aren't enough valid vouches.

[clesaege]

This is a while loop (while (request.vouches.length < requiredVouches)), so it will continue to run while the condition is satisfied. Therefore it will stop running when the condition stops being satisfied, thus when !(request.vouches.length < requiredVouches) which is equivalent to request.vouches.length >= requiredVouches. So it will stop when the required amount of vouches has been reached.

If you believe my answer to be incorrect, please provide a test showing that.