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

Proof of Humanity Protocol v2
2 stars 1 forks source link

`renewHumanity` can be called earlier than expected #80

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

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

Github username: -- Twitter username: -- Submission hash (on-chain): 0x86f2a9c51c787f945bd37203e1440c61c18eff56e5fd58ea0fc597c83d0b0c3d Severity: low

Description: Description\ The expirationTime in renewHumanity will be forever true when uint40(block.timestamp) overflows. As a result, the function renewHumanity will be callable before the desired date.

Reference to same bug, different issue: https://solodit.xyz/issues/m-07-vesting-schedule-start-and-end-time-can-be-set-in-the-past-code4rena-vtvl-vtvl-contest-git

  1. Proof of Concept

ProofOfHumanity.sol@L741-758

function renewHumanity(string calldata _evidence) external payable {
    bytes20 humanityId = accountHumanity[msg.sender];

    Humanity storage humanity = humanityData[humanityId];

    require(humanity.owner == msg.sender);
@>  require(humanity.expirationTime.subCap40(renewalPeriodDuration) < block.timestamp);

    uint256 requestId = _requestHumanity(humanityId);

    emit RenewalRequest(msg.sender, humanityId, requestId);
    emit Evidence(
        arbitratorDataHistory[arbitratorDataHistory.length - 1].arbitrator,
        uint256(keccak256(abi.encodePacked(humanityId, requestId))),
        msg.sender,
        _evidence
    );
}

ProofOfHumanity.sol@L1157-1182

function executeRequest(bytes20 _humanityId, uint256 _requestId) external {
    Humanity storage humanity = humanityData[_humanityId];
    Request storage request = humanity.requests[_requestId];
    require(request.status == Status.Resolving);
    require(request.challengePeriodStart + challengePeriodDuration < block.timestamp);

    if (request.revocation) {
        delete humanity.owner;
        humanity.pendingRevocation = false;

        emit HumanityRevoked(_humanityId, _requestId);
    } else if (!request.punishedVouch) {
        humanity.owner = request.requester;
@>      humanity.expirationTime = uint40(block.timestamp).addCap40(humanityLifespan);

        emit HumanityClaimed(_humanityId, _requestId);
    }

    humanity.nbPendingRequests--;
    request.status = Status.Resolved;
    delete humanity.requestCount[request.requester];

    if (request.vouches.length != 0) processVouches(_humanityId, _requestId, VOUCHES_TO_AUTOPROCESS);

    withdrawFeesAndRewards(request.requester, _humanityId, _requestId, 0, 0); // Automatically withdraw for the requester.
}
  1. Revised Code File

Cast the block.timestamp to uint40.

ProofOfHumanity.sol@L741-758

function renewHumanity(string calldata _evidence) external payable {
    bytes20 humanityId = accountHumanity[msg.sender];

    Humanity storage humanity = humanityData[humanityId];

    require(humanity.owner == msg.sender);
--  require(humanity.expirationTime.subCap40(renewalPeriodDuration) < block.timestamp);
++  require(humanity.expirationTime.subCap40(renewalPeriodDuration) < uint40(block.timestamp));

    uint256 requestId = _requestHumanity(humanityId);

    emit RenewalRequest(msg.sender, humanityId, requestId);
    emit Evidence(
        arbitratorDataHistory[arbitratorDataHistory.length - 1].arbitrator,
        uint256(keccak256(abi.encodePacked(humanityId, requestId))),
        msg.sender,
        _evidence
    );
}
clesaege commented 2 weeks ago

This will happen in the year 2^40 / (3652460*60) + 1970 = 36835, in the meantime we will very likely have multiple versions of Proof Of Humanity (contracts are upgradable through governance).

The issue is excluded by the contest rule: Any issue that is only theoretical but can't happen in practice.