code-423n4 / 2024-05-arbitrum-foundation-findings

3 stars 2 forks source link

confirmation timelock grace period is bypassed when the contract is paused and then unpaused #45

Open howlbot-integration[bot] opened 6 months ago

howlbot-integration[bot] commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/rollup/RollupUserLogic.sol#L125

Vulnerability details

Impact

confirmation timelock is bypassed when the contract is paused and then unpaused

Proof of Concept

After a new assertion is created, there is a timelock period

// Check that deadline has passed
require(block.number >= assertion.createdAtBlock + prevConfig.confirmPeriodBlocks, "BEFORE_DEADLINE");

the prevConfig.confirmPeriodBlocks is the timelock window that act as buffer so the assertion cannot be confirmed immeidately.

However, when the contract the paused, the new block still get mined and increment the block.number

when the contract is unpaused, the assertion can be immediately confirmed but trhe timelock period prevConfig.confirmPeriodBlocks is not honored.

the challange grace period will be bypassed as well in the cause when contract is paused, leaving no time to challange a invalid assertion.

if (prevAssertion.secondChildBlock > 0) {
        // if the prev has more than 1 child, check if this assertion is the challenge winner
        ChallengeEdge memory winningEdge = IEdgeChallengeManager(prevConfig.challengeManager).getEdge(winningEdgeId);
        require(winningEdge.claimId == assertionHash, "NOT_WINNER");
        require(winningEdge.status == EdgeStatus.Confirmed, "EDGE_NOT_CONFIRMED");
        require(winningEdge.confirmedAtBlock != 0, "ZERO_CONFIRMED_AT_BLOCK");
        // an additional number of blocks is added to ensure that the result of the challenge is widely
        // observable before it causes an assertion to be confirmed. After a winning edge is found, it will
        // always be challengeGracePeriodBlocks before an assertion can be confirmed
        require(
            block.number >= winningEdge.confirmedAtBlock + challengeGracePeriodBlocks,
            "CHALLENGE_GRACE_PERIOD_NOT_PASSED"
        );
    }

Tools Used

Manual Review

Recommended Mitigation Steps

consider extend grace period for resolve assertion and edge if the contract is unpaused after paused.

Assessed type

Token-Transfer

c4-judge commented 5 months ago

Picodes marked the issue as satisfactory

c4-judge commented 5 months ago

Picodes changed the severity to QA (Quality Assurance)