The top tier prover can not re-prove

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-03-taiko/blob/main/packages/protocol/contracts/L1/libs/LibProving.sol#L219-L236 https://github.com/code-423n4/2024-03-taiko/blob/main/packages/protocol/contracts/L1/libs/LibProving.sol#L389

Vulnerability details

Impact

In the LibProving.proveBlock function the top tier prover is allowed to prove a new transition as long as the new transition is different from the previous transition and assert conditional checks pass.

            if (sameTransition) revert L1_ALREADY_PROVED();

            if (isTopTier) {
                // The top tier prover re-proves.
                assert(tier.validityBond == 0);
                assert(ts.validityBond == 0 && ts.contestBond == 0 && ts.contester == address(0));

But the assert condition of this logic is wrong since it checks for the ts.contestBond == 0 where as it should be ts.contestBond == 1 since 1 is set as the default value for ts.contestBond parameter for gas savings as shown below:

            ts_.contestBond = 1; // to save gas

As a result of the even though code expects the top-tier prover to re-prove a different transition, the transaction will revert.

Proof of Concept

Add the following testcase test_L1_GuardianProverCanOverwriteIfNotSameProof_test to the TaikoL1LibProvingWithTiers.t.sol test file.

If you change the ts.contestBond == 0 to ts.contestBond == 1 in the second assert statement of the LibProving.proveBlock function, the test will run successfully and transaction execution will succeed.

    function test_L1_GuardianProverCanOverwriteIfNotSameProof_test() external {
        giveEthAndTko(Alice, 1e7 ether, 1000 ether);
        giveEthAndTko(Carol, 1e7 ether, 1000 ether);
        console2.log("Alice balance:", tko.balanceOf(Alice));
        // This is a very weird test (code?) issue here.
        // If this line is uncommented,
        // Alice/Bob has no balance.. (Causing reverts !!!)
        // Current investigations are ongoing with foundry team
        giveEthAndTko(Bob, 1e7 ether, 100 ether);
        console2.log("Bob balance:", tko.balanceOf(Bob));
        // Bob
        vm.prank(Bob, Bob);

        bytes32 parentHash = GENESIS_BLOCK_HASH;
        for (uint256 blockId = 1; blockId < conf.blockMaxProposals * 3; blockId++) {
            printVariables("before propose");
            (TaikoData.BlockMetadata memory meta,) = proposeBlock(Alice, Bob, 1_000_000, 1024);
            //printVariables("after propose");
            mine(1);

            bytes32 blockHash = bytes32(1e10 + blockId);
            bytes32 stateRoot = bytes32(1e9 + blockId);
            // This proof cannot be verified obviously because of
            // blockhash:blockId
            proveBlock(Bob, Bob, meta, parentHash, stateRoot, stateRoot, LibTiers.TIER_GUARDIAN, "");

            // Prove as guardian                       
            proveBlock(
                Carol, Carol, meta, parentHash, blockHash, stateRoot, LibTiers.TIER_GUARDIAN, ""
            );

            vm.roll(block.number + 15 * 12);

            uint16 minTier = meta.minTier;
            vm.warp(block.timestamp + tierProvider().getTier(LibTiers.TIER_GUARDIAN).cooldownWindow * 60 + 1);

            verifyBlock(Carol, 1);

            parentHash = blockHash;
        }
        printVariables("");
    }

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence recommended to update the ts.contestBond == 0 in the second assert statement to ts.contestBond == 1 in the LibProving.proveBlock function.

Assessed type

DoS

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-pre-sort]

minhquanym marked the issue as sufficient quality report

[dantaik]

This is a valid bug report, it has been fixed already here: https://github.com/taikoxyz/taiko-mono/pull/16543

[c4-sponsor]

dantaik (sponsor) confirmed

[c4-judge]

0xean marked the issue as satisfactory

[c4-judge]

0xean marked the issue as selected for report

[genesiscrew]

Hi @0xean! Just like to highlight a few things here, even though this report does provide a coded POC, it does not provide context or show impact of the bug on the protocol as is provided by the two reports marked as duplicates, one of which is mine. Those two reports provided some context and explained in detail via code snippets the issue and its impact on protocol.

Furthermore I am uncertain whether Medium is sufficient here, my report was marked as high because of the context of this bug. In the case that the top tier prover cannot reprove a block because its post state was deemed invalid by offchain validations, the state of the blockchain will remain invalid which could cause a whole host of problems, where one can imagine a potential consequence of invalid states is loss of funds.

Thanks!

[0xean]

leaving this as judged. I don't see any report showing a direct loss of funds.