hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

1 stars 0 forks source link

Missing Arbitration State Check in stateOpenOrPendingArbitration Modifier #6

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

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

Description: Description

The stateOpenOrPendingArbitration modifier does not accurately reflect its intended functionality, which could lead to unexpected behavior. Despite its name suggesting that it allows both "open" and "pending arbitration" states, the modifier does not include a specific check for the "pending arbitration" state. This could result in functions protected by this modifier being callable only when the question is in an "open" state, potentially bypassing the intended restriction for arbitration-related actions.

Attachments

  1. Proof of Concept (PoC)
modifier stateOpenOrPendingArbitration(bytes32 question_id) {
        require(questions[question_id].timeout > 0, "question must exist");
        uint32 finalize_ts = questions[question_id].finalize_ts;
        require(finalize_ts == UNANSWERED || finalize_ts > uint32(block.timestamp), "finalization dealine must not have passed");
        uint32 opening_ts = questions[question_id].opening_ts;
        require(opening_ts == 0 || opening_ts <= uint32(block.timestamp), "opening date must have passed"); 
        _;
    }
  1. Revised Code

Add a check for the "pending arbitration" state within the modifier to ensure that the protected functions are accessible only when the question is either open or undergoing arbitration.

modifier stateOpenOrPendingArbitration(bytes32 question_id) {
        require(questions[question_id].timeout > 0, "question must exist");
        uint32 finalize_ts = questions[question_id].finalize_ts;
---     require(finalize_ts == UNANSWERED || finalize_ts > uint32(block.timestamp), "finalization dealine must not have passed");
+++     require(
+++     (finalize_ts == UNANSWERED || finalize_ts > uint32(block.timestamp)) || 
+++     questions[question_id].is_pending_arbitration, 
+++     "question must be open or pending arbitration"
+++ );
        uint32 opening_ts = questions[question_id].opening_ts;
        require(opening_ts == 0 || opening_ts <= uint32(block.timestamp), "opening date must have passed"); 
        _;
}
clesaege commented 1 month ago

This modifier is only used when revealing an answer made with commit and reveal. I do understand that the name is not perfect as this modifier could prevent the access to a function when arbitration has started but after the theoretical deadline to correct the answer has passed.

However the way it is used, it's not a problem, because when finalize_ts<block.timestamp, it would be too late to reveal.

According to contest rules, are excluded: