hats-finance / Proof-Of-Humanity-V2-0xef0709445d394a22704850c772a28a863bb780b0

Proof of Humanity Protocol v2
2 stars 1 forks source link

Challenge made for `IncorrectSubmission` can not be peanised #124

Open hats-bug-reporter[bot] opened 2 months ago

hats-bug-reporter[bot] commented 2 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xf2fa81cd6731ee162d3b65ed8cc74363f28a44dcc15616807533f80588cacded Severity: high

Description: Description\

Once the Humanity is requested, funded and reaches the resolving state, it can be challenged for following reason.

ProofOfHumanity.sol#L64-L70

    enum Reason {
        None, // No reason specified. This option should be used to challenge removal requests.
        IncorrectSubmission, // Request does not comply with the rules.
        IdentityTheft, // Attempt to claim the humanity ID of another human.
        SybilAttack, // Duplicate or human does not exist.
        Deceased // Human has existed but does not exist anymore.
    }

Except the first and last, other three reason can be penalised when they challenged.

Using the function challengeRequest , the request can be challenged with any one of the above reason.

Later in the function rule will update the request.ultimateChallenger if the dispute is won.

After that the function processVouches can be called with input index value.

Following line can be seen processVouches

ProofOfHumanity.sol#L1208-L1210

        // Penalty is applied for sybil attacks.
        bool applyPenalty = request.ultimateChallenger != address(0x0) &&
            (currentReason == Reason.SybilAttack || currentReason == Reason.IdentityTheft);

So if the challenge is made for IncorrectSubmission, the applyPenalty can not be updated.

ProofOfHumanity.sol#L1212-L1229

        while (lastProcessed < endIndex) {
            Humanity storage voucherHumanity = humanityData[request.vouches[lastProcessed]];
            voucherHumanity.vouching = false;

            if (applyPenalty) {
                // Situation when vouching address is in the middle of renewal process.
                uint256 voucherRequestId = voucherHumanity.requestCount[voucherHumanity.owner] - 1;
                if (voucherRequestId != 0) voucherHumanity.requests[voucherRequestId].punishedVouch = true;

                delete voucherHumanity.owner;

                emit HumanityDischargedDirectly(request.vouches[lastProcessed]);
            }

            unchecked {
                lastProcessed++;
            }
        }

so, the punishedVouch can not be updated.

When executing the request using executeRequest the request with incorrect information can be passed and himanity can be claimed.

ProofOfHumanity.sol#L1168-L1173

        } else if (!request.punishedVouch) {
            humanity.owner = request.requester;
            humanity.expirationTime = uint40(block.timestamp).addCap40(humanityLifespan);

            emit HumanityClaimed(_humanityId, _requestId);
        }

Attack Scenario\

Humanity with incorrect or fake information can be accepted and claimed.

  1. Revised Code File (Optional)

when processVouches, consider the IncorrectSubmission value also to apply the penalty.

clesaege commented 2 months ago

The applyPenalty is to penalize vouchers of malicious submissions. An incorrect submission is just rejected without penalizing vouchers. If you believe there are cases where the humanity is still created despite it losing an incorrect submission challenge, please provide and example of such case.

aktech297 commented 2 months ago

The applyPenalty is to penalize vouchers of malicious submissions. An incorrect submission is just rejected without penalizing vouchers. If you believe there are cases where the humanity is still created despite it losing an incorrect submission challenge, please provide and example of such case.

Hey @clesaege why incorrect submission are not penalized ?

Incorrect submission also malicious so they should be penalized.

The challenge function accepts any type of challenge as mentioned above.

It is fair to check for incorrect submission. because somebody is going to use some other's identity to claim the humanity. The arbitration court should consider this point also.

clesaege commented 2 months ago

Hey @clesaege why incorrect submission are not penalized ?

Incorrect submission just means that the submission is breaking some rules, it doesn't mean that it's a Sybil attack. Removing people from vouching to someone doing honest mistakes would be too harsh.

aktech297 commented 2 months ago

Hey @clesaege why incorrect submission are not penalized ?

Incorrect submission just means that the submission is breaking some rules, it doesn't mean that it's a Sybil attack. Removing people from vouching to someone doing honest mistakes would be too harsh.

can you explain, why breaking rules is good behavior ? The case is not always honest mistake, if they request by mistake, there are functions to withdraw the request. But, the cases mentioned here can have both cases. honest or dishonest. In real world, any incorrect submission with malicious intention can be penalized. that's how the human judging system works. For submission made with dishonest intention, they challenger can make challenge and provide the evidence. But this is not accepted in the current design. so dishonest submitter is going through to claim the humanity.

clesaege commented 2 months ago

Incorrect submission is stuff like "The user only showed 15 characters of his address" or "His cat was hiding part of the internal features of his face" or the "the camara quality was so low that we can't read the address". For all those, losing a deposit is enough.

aktech297 commented 2 months ago

Incorrect submission is stuff like "The user only showed 15 characters of his address" or "His cat was hiding part of the internal features of his face" or the "the camara quality was so low that we can't read the address". For all those, losing a deposit is enough.

Didn't get you on this .. sorry

clesaege commented 2 months ago

The whole point of the "Incorrect Submission" is to have a challenge reason which does not lead to removal of vouchers. This is used for reasons which are not expected to be an attack (I gave you some examples in my previous post). You can refer to the registration rules.