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

Proof of Humanity Protocol v2
2 stars 1 forks source link

Potential Stale Data in `getHumanityInfo` #27

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

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

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

Description: Description\ The getHumanityInfo function in the ProofOfHumanity.sol file directly returns data, like the owner of the humanity without checking if the humanity's expirationTime has passed. This could lead to a situation where the function returns an outdated or invalid owner, as it does not account for the humanity potentially being expired.

In contrast, the boundTo function checks whether the current timestamp is less than the expirationTime before returning the owner, and returns the zero address if the humanity has expired.

This inconsistency can cause issues where external systems or users rely on the getHumanityInfo function, as it may return stale or incorrect data about the humanity's ownership.

  1. Proof of Concept (PoC)

in ProofOfHumanity.sol:

function getHumanityInfo(
    bytes20 _humanityId
)
    external
    view
    override
    returns (
        bool vouching,
        bool pendingRevocation,
        uint48 nbPendingRequests,
        uint40 expirationTime,
        address owner,
        uint256 nbRequests
    )
{
    Humanity storage humanity = humanityData[_humanityId];
    return (
        humanity.vouching,
        humanity.pendingRevocation,
        humanity.nbPendingRequests,
        humanity.expirationTime,
    @>  humanity.owner,
        humanity.requests.length
        );
    }
function boundTo(bytes20 _humanityId) external view override returns (address) {
    Humanity storage humanity = humanityData[_humanityId];
@>  return block.timestamp < humanity.expirationTime ? humanity.owner : address(0x0);
}
  1. Revised Code

Consider implementing additional logic to prevent the presence of stale or incorrect data and updating the data if necessary.

clesaege commented 2 weeks ago

getHumanityInfo is a simple getter, simply returning the struct, which is used for the front. This is the expected behaviour. boundTo is, per the comments, returning "the owner of a humanity." which if a submission has expired will be no one. This is the expected behaviour.