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

1 stars 2 forks source link

Assertions can take longer than ~14 days to confirm #73

Open howlbot-integration[bot] opened 1 month ago

howlbot-integration[bot] commented 1 month ago

Lines of code

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

Vulnerability details

Description

One of the assertions is:

- Honest assertions / layer zero edges are always confirmed under the BoLD paper’s assumptions within 2 challenge periods.
    - plus the security council grace period for assertions

As per the BoLD paper, honest assertions should be confirmed at most ~14 days. However, it is possible to pause the ability to confirm assertions by calling RollupAdminLogic.pause()

    function pause() external override {
        _pause();
        emit OwnerFunctionCalled(3);
    }

If this function gets called, any call made to RollupUserLogic.confirmAssertion() will fail due to the modifier whenNotPaused:

    function confirmAssertion(
        bytes32 assertionHash,
        bytes32 prevAssertionHash,
        AssertionState calldata confirmState,
        bytes32 winningEdgeId,
        ConfigData calldata prevConfig,
        bytes32 inboxAcc
->  ) external onlyValidator whenNotPaused {

This means that there are scenario's possible where an assertion will take longer than the ~14 days described in the BoLD paper, thus breaking this main invariant.

Tools used

Manual Review

Recommended Mitigation Steps

Let the time run during a pause() to stop it from breaking the main invariant.

Assessed type

Other

c4-judge commented 1 month ago

Picodes changed the severity to QA (Quality Assurance)

c4-judge commented 1 month ago

This previously downgraded issue has been upgraded by Picodes

c4-judge commented 1 month ago

This previously downgraded issue has been upgraded by Picodes

c4-judge commented 1 month ago

Picodes marked the issue as duplicate of #7

c4-judge commented 1 month ago

Picodes changed the severity to QA (Quality Assurance)

c4-judge commented 1 month ago

Picodes marked the issue as grade-b

bronzepickaxe commented 1 month ago

Hi @Picodes,

Finding-55 has been upgraded to a Medium while only showing an invariant been broken.

Finding-55 does not outline a loss for the user, solely a small delay wrt the delay buffer. Can you revisit this finding group in the light of your new judgement? Since this finding group provides an example of an invariant being broken, which has been explicitly mentioned in the README.md.

Picodes commented 1 month ago

@bronzepickaxe 55 is about funds being stuck longer than they should, without any admin intervention, so it's not related to this group of reports