search

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

0 stars 0 forks source link

Lack of validation of parent assertion existence in `fastConfirmAssertion` function #342

Open c4-bot-1 opened 6 months ago

c4-bot-1 commented 6 months ago

Lines of code

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

Vulnerability details

Impact

anyTrustFastConfirmer is supposed to be set only on an AnyTrust chain, it can force confirm any pending assertion, this is a feature which allows a committee members (multi-sig) to confirm assertions quicker.

This can be done by calling any of those two functions:

fastConfirmNewAssertion checks if the parent assertion exists. However, fastConfirmAssertion doesn't do that.

Not sure how much anyTrustFastConfirmer is trusted, but I believe it shouldn't be allowed to create a completely new assertion chain based on a wrong parent assertion hash. If this happens, it breaks BoLD assumptions by having only one correct assertion chain, and not being able to go back in history of assertions till the genesis assertion.

Proof of Concept

in fastConfirmNewAssertion there is a check

getAssertionStorage(prevAssertion).requireExists();

RollupUserLogic.sol:L286-L287

However, there is none in fastConfirmAssertion

    function fastConfirmAssertion(
        bytes32 assertionHash,
        bytes32 parentAssertionHash,
        AssertionState calldata confirmState,
        bytes32 inboxAcc
    ) public whenNotPaused {
        require(msg.sender == anyTrustFastConfirmer, "NOT_FAST_CONFIRMER");
        // this skip deadline, prev, challenge validations
        confirmAssertionInternal(assertionHash, parentAssertionHash, confirmState, inboxAcc);
    }

RollupUserLogic.sol:L252-L262

Tools Used

Manual analysis

Recommended Mitigation Steps

Add this check getAssertionStorage(prevAssertion).requireExists(); in fastConfirmAssertion function.

Assessed type

Other

koolexcrypto commented 5 months ago

This action is done by a trusted role but there is a lack of validating the parent assertion existence which is done in fastConfirmNewAssertion

Could you please clarify why this was deemed invalid ?

Thank you!

godzillaba commented 5 months ago

hi @koolexcrypto , fastConfirmAssertion doesn't need to check that the parent exists because it is confirming an already existing assertion. when assertions are created it checks that their parent exists

koolexcrypto commented 5 months ago

hi @koolexcrypto , fastConfirmAssertion doesn't need to check that the parent exists because it is confirming an already existing assertion. when assertions are created it checks that their parent exists

Makes sense. Thank you!