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

1 stars 2 forks source link

Lack of input validation in edge pool allows malicious user to create a clearly invalid insertion to make staker lose fund. #44

Closed howlbot-integration[bot] closed 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/assertionStakingPool/EdgeStakingPool.sol#L44

Vulnerability details

Impact

Lack of input validation in edge pool allows malicious user to create a clearly invalid insertion to make staker lose fund.

Proof of Concept

anyone can create an edge pool with edge id

  constructor(
        address _challengeManager,
        bytes32 _edgeId
    ) AbsBoldStakingPool(address(EdgeChallengeManager(_challengeManager).stakeToken())) {
        challengeManager = _challengeManager;
        edgeId = _edgeId;
    }

then after there are enough user stake fund, an edge can be created.

 function createEdge(CreateEdgeArgs calldata args) external {
        uint256 requiredStake = EdgeChallengeManager(challengeManager).stakeAmounts(args.level);
        IERC20(stakeToken).safeIncreaseAllowance(address(challengeManager), requiredStake);
        bytes32 newEdgeId = EdgeChallengeManager(challengeManager).createLayerZeroEdge(args);
        if (newEdgeId != edgeId) {
            revert IncorrectEdgeId(newEdgeId, edgeId);
        }
    }

the function takes arbitrary CreateEdgeArgs input:

/// @notice Data for creating a layer zero edge
struct CreateEdgeArgs {
    /// @notice The level of edge to be created. Challenges are decomposed into multiple levels.
    ///         The first (level 0) being of type Block, followed by n (set by NUM_BIGSTEP_LEVEL) levels of type BigStep, and finally
    ///         followed by a single level of type SmallStep. Each level is bisected until an edge
    ///         of length one is reached before proceeding to the next level. The first edge in each level (the layer zero edge)
    ///         makes a claim about an assertion or assertion in the lower level.
    ///         Finally in the last level, a SmallStep edge is added that claims a lower level length one BigStep edge, and these
    ///         SmallStep edges are bisected until they reach length one. A length one small step edge
    ///         can then be directly executed using a one-step proof.
    uint8 level;
    /// @notice The end history root of the edge to be created
    bytes32 endHistoryRoot;
    /// @notice The end height of the edge to be created.
    /// @dev    End height is deterministic for different levels but supplying it here gives the
    ///         caller a bit of extra security that they are supplying data for the correct level of edge
    uint256 endHeight;
    /// @notice The edge, or assertion, that is being claimed correct by the newly created edge.
    bytes32 claimId;
    /// @notice Proof that the start history root commits to a prefix of the states that
    ///         end history root commits to
    bytes prefixProof;
    /// @notice Edge type specific data
    ///         For Block type edges this is the abi encoding of:
    ///         bytes32[]: Inclusion proof - proof to show that the end state is the last state in the end history root
    ///         AssertionStateData: the before state of the edge
    ///         AssertionStateData: the after state of the edge
    ///         bytes32 predecessorId: id of the prev assertion
    ///         bytes32 inboxAcc:  the inbox accumulator of the assertion
    ///         For BigStep and SmallStep edges this is the abi encoding of:
    ///         bytes32: Start state - first state the edge commits to
    ///         bytes32: End state - last state the edge commits to
    ///         bytes32[]: Claim start inclusion proof - proof to show the start state is the first state in the claim edge
    ///         bytes32[]: Claim end inclusion proof - proof to show the end state is the last state in the claim edge
    ///         bytes32[]: Inclusion proof - proof to show that the end state is the last state in the end history root
    bytes proof;
}

there is lack of input validation

note, when create layerzero edge, the parameter claimId is not sufficiently validated:

  function createLayerZeroEdge(
        EdgeStore storage store,
        CreateEdgeArgs calldata args,
        AssertionReferenceData memory ard,
        IOneStepProofEntry oneStepProofEntry,
        uint256 expectedEndHeight,
        uint8 numBigStepLevel,
        bool whitelistEnabled
    ) internal returns (EdgeAddedData memory) {
        // each edge type requires some specific checks
        (ProofData memory proofData, bytes32 originId) =
            layerZeroTypeSpecificChecks(store, args, ard, oneStepProofEntry, numBigStepLevel);
        // all edge types share some common checks
        (bytes32 startHistoryRoot) = layerZeroCommonChecks(proofData, args, expectedEndHeight);
        // we only wrap the struct creation in a function as doing so with exceeds the stack limit
        ChallengeEdge memory ce = toLayerZeroEdge(originId, startHistoryRoot, args);

this is calling:

layerZeroTypeSpecificChecks

basically if the levelToType is EdgeType.block, the claimId needs equal to a a pending and has sibling assertion references.

// if the assertion is already confirmed or rejected then it cant be referenced as a claim
if (!ard.isPending) {
    revert AssertionNotPending();
}

// if the claim doesnt have a sibling then it is undisputed, there's no need
// to open challenge edges for it
if (!ard.hasSibling) {
    revert AssertionNoSibling();
}

if a user knows that the pending assertion will be rejected, he will use that assertion to create an edge to make sure the edge is not confirmed as well.

if the levelToType is not EdgeType.block, the code requies the claimEdge is in pending status as well:

// hasLengthOneRival checks existance, so we can use getNoCheck
ChallengeEdge storage claimEdge = getNoCheck(store, args.claimId);

// origin id is the mutual id of the claim
// all rivals and their children share the same origin id - it is a link to the information
// they agree on
bytes32 originId = claimEdge.mutualId();

// once a claim is confirmed it's status can never become pending again, so there is no point
// opening a challenge that references it
if (claimEdge.status != EdgeStatus.Pending) {
    revert ClaimEdgeNotPending();
}

a invalid pending edge will not be confirmed,

if a user knows that the running edge will be rejected, he will use that edge id (claim id) to create an edge to make sure the edge is not confirmed as well.

Tools Used

Manual Review

Recommended Mitigation Steps

add add control to createEdge functoin

or when the edge staking pool is deployed, set the parameter in the constructor

CreateEdgeArgs calldata args

so the staker can research and make sure the args is valid and the the staker can stake fund.

Assessed type

Invalid Validation

c4-judge commented 1 month ago

Picodes marked the issue as unsatisfactory: Invalid