Lines of code

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/main/src/challengeV2/libraries/EdgeChallengeManagerLib.sol#L528 https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/main/src/challengeV2/libraries/EdgeChallengeManagerLib.sol#L698-L700

Vulnerability details

Impact

Adversary will have his edge confirmed, even though he wasn't able to prove his single step edge.

Proof of Concept

updateTimerCacheByClaim only checks that the mutualId of edgeId==originId of claimingEdge. Since mutualId of two rivals are equal, adversary can updateTimerCacheByClaim, entering the claiming (or one step proved) edge of his rival:

updateTimerCacheByClaim function:

function updateTimerCacheByClaim(
    EdgeStore storage store,
    bytes32 edgeId,
    bytes32 claimingEdgeId,
    uint8 numBigStepLevel
) internal returns (bool, uint256) {
    // calculate the time unrivaled without inheritance
    uint256 totalTimeUnrivaled = timeUnrivaled(store, edgeId);
@>  checkClaimIdLink(store, edgeId, claimingEdgeId, numBigStepLevel); //@audit-issue I can update my timerCache with my rival's claim timer
    totalTimeUnrivaled += store.edges[claimingEdgeId].totalTimeUnrivaledCache; 
    return updateTimerCache(store, edgeId, totalTimeUnrivaled);
}

checkClaimIdLink function:

    function checkClaimIdLink(
        EdgeStore storage store,
        bytes32 edgeId,
        bytes32 claimingEdgeId,
        uint8 numBigStepLevel
    ) private view {
@>      if (store.edges[edgeId].mutualId() != store.edges[claimingEdgeId].originId) {
            revert OriginIdMutualIdMismatch(
                store.edges[edgeId].mutualId(),
                store.edges[claimingEdgeId].originId
            );
        }
        // the claiming edge must be exactly one level below
        if (
            nextEdgeLevel(store.edges[edgeId].level, numBigStepLevel) !=
            store.edges[claimingEdgeId].level
        ) {
            revert EdgeLevelInvalid(
                edgeId,
                claimingEdgeId,
                nextEdgeLevel(store.edges[edgeId].level, numBigStepLevel),
                store.edges[claimingEdgeId].level
            );
        }
    }

If an adversary cannot prove his edge, but an honest party has, he would update his timerCache with the honest party's. This will allow him to confirm his edge.

Tools Used

Manual Review

Recommended Mitigation Steps

Within the ChallengeEdge struct, there should be a parameter that does not change across levels and bisections of an edge. In the current implementation, claimId is set to 0 when bisecting an edge. This is why we can't use it as the "parameter" we are looking for. I suggest having a parameter ,let's say uniqueID(I'm short of names) within the challenge Edge struct. This "uniqueID" should be set to claimId of the block edge being created. The difference between ChallengeEdge.claimId and ChallengeEdge.uniqueId is that while the former is set to 0 during edge bisection, the latter will not change.

Now, within checkClaimIdLink function, also check that :

    function checkClaimIdLink(
        EdgeStore storage store,
        bytes32 edgeId,
        bytes32 claimingEdgeId,
        uint8 numBigStepLevel
    ) private view {
        if (store.edges[edgeId].mutualId() != store.edges[claimingEdgeId].originId) {
            revert OriginIdMutualIdMismatch(
                store.edges[edgeId].mutualId(),
                store.edges[claimingEdgeId].originId
            );
        }
@>      if (store.edges[edgeId].uniqueId != store .edges[claimingEdgeId].uniqueId){
            revert("UniqueId must match");
        }
        // the claiming edge must be exactly one level below
        if (
            nextEdgeLevel(store.edges[edgeId].level, numBigStepLevel) !=
            store.edges[claimingEdgeId].level
        ) {
            revert EdgeLevelInvalid(
                edgeId,
                claimingEdgeId,
                nextEdgeLevel(store.edges[edgeId].level, numBigStepLevel),
                store.edges[claimingEdgeId].level
            );
        }
    }

Assessed type

Context

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

Adversary can update his timerCache with his rival's(i.e. honest party's) timer, and maliciously win assertions #27