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

1 stars 2 forks source link

Stakers can create edges with non unique Edge ID's. #52

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#L47 https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/challengeV2/EdgeChallengeManager.sol#L447

Vulnerability details

Impact

Stakers can create edges with non unique Edge ID's which breaks the protocol's main invariant.

Proof of Concept

One of the main invariants of the protocol is that Edge ID's and Assertion ID’s must be unique. This isn't entirely true, as duplicate edges with the same edge id can be created. This breaks one of the main invariants of the protocol. The bug sterns from the fact that the there is no check in place to ensure that the edge id of the new edge that is about to created is unique (i.e it hasn't been created before with those parameters). Duplicate edges will have the same edge Id, which it to be non unique. See Additonal context / Main Invariants https://code4rena.com/audits/2024-05-arbitrum-bold

Main invariants
...
Edge ID’s and Assertion ID’s must be unique

When two assertions are created that have the same predecessor the protocol needs to decide which of the two is correct. Stakers then compete by creating edges that reference the assertion that they believe in. If stakers create two or more edges that are referencing the same assertion, the edge ids of those edges will not be unique.

The following test can be added to EdgeStakingPool.t.sol to demonstrate the described scenario.

    function testCreateEdgesWithNonUniqueEdgeIDs(CreateEdgeArgs memory args) public {
        uint256 requiredStake = challengeManager.stakeAmounts(args.level);
        bytes32 realEdgeId = keccak256(abi.encode(args));
        IEdgeStakingPool stakingPool = stakingPoolCreator.createPool(address(challengeManager), realEdgeId);

        token.transfer(address(stakingPool), requiredStake);
        stakingPool.createEdge(args);

        // Creating another edge with the same args in order to make the edge id to be non unique
        token.transfer(address(stakingPool), requiredStake);
        stakingPool.createEdge(args);

        assertEq(token.balanceOf(address(stakingPool)), 0);
        assertEq(token.balanceOf(address(challengeManager)), 2 * requiredStake);
    }

The two edges will have the same edge id, making it to be non unique to an edge.

Edge ID 1: 0x6e3776c7656560aeae3a4a471cacf76bc8ee0f2d96eb74e2a8d2c3b4ad463f14
Edge ID 2: 0x6e3776c7656560aeae3a4a471cacf76bc8ee0f2d96eb74e2a8d2c3b4ad463f14

Tools Used

Manual Review

Recommended Mitigation Steps

The protocol should check if the edge id of the new edge to be created is unique and it should revert if it is not unique.

Assessed type

Invalid Validation

c4-sponsor commented 1 month ago

gzeoneth (sponsor) disputed

gzeoneth commented 1 month ago

POC only worked with the mock, actual implementation will revert; also, edges with the same args is the same edge

c4-judge commented 1 month ago

Picodes marked the issue as unsatisfactory: Invalid