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

Proof of Humanity Protocol v2
2 stars 1 forks source link

Bypass of Challenge Period Through Duration Change #89

Closed FeeqCodes closed 1 month ago

FeeqCodes commented 1 month ago

Description\ The changeDurations function in the ProofOfHumanity contract allows the governor to modify critical time parameters, including the challengePeriodDuration. This presents a vulnerability where the governor could potentially manipulate the challenge period for ongoing requests, bypassing the intended security measures.

Attack Scenario\ When a request is submitted, it enters a challenge period during which it can be contested. The duration of this period is crucial for maintaining the integrity of the system. However, if the governor changes the challengePeriodDuration while requests are in the challenge period, it could prematurely end or extend the challenge period for those requests.

This vulnerability could be exploited in two ways:

Shortening the challenge period: The governor could drastically reduce the challengePeriodDuration, potentially allowing requests to pass through with insufficient time for proper scrutiny. Extending the challenge period: The governor could indefinitely extend the challengePeriodDuration, effectively freezing requests in the challenge state. Either scenario compromises the fairness and predictability of the challenge process, potentially leading to the acceptance of fraudulent requests or the unfair blocking of legitimate ones.

POC

     function changeDurations( // @audit if the governor changes the duration, this would enable a bypass for request which are currently in the challenge period
        uint40 _humanityLifespan,
        uint40 _renewalPeriodDuration,
        uint40 _challengePeriodDuration,
        uint40 _failedRevocationCooldown
    ) external onlyGovernor {
        humanityLifespan = _humanityLifespan;
        renewalPeriodDuration = _renewalPeriodDuration;
        challengePeriodDuration = _challengePeriodDuration;
        failedRevocationCooldown = _failedRevocationCooldown;
        emit DurationsChanged(
            _humanityLifespan,
            _renewalPeriodDuration,
            _challengePeriodDuration,
            _failedRevocationCooldown
        );
    }

Recommendation

  1. Introduce a time delay for duration changes: Implement a timelock mechanism where changes to durations only take effect after a predetermined period, allowing users to be aware of upcoming changes.

  2. Use a separate storage for active requests: Store the challenge period end time for each request at the time of submission, making it independent of subsequent duration changes.

clesaege commented 1 month ago

"This presents a vulnerability where the governor could potentially manipulate the challenge period for ongoing requests, bypassing the intended security measures." The governor is trusted and will be moved to the governance contract.

As per the contest rules are excluded: