Skip to content

Latest commit

 

History

History
84 lines (60 loc) · 3.42 KB

092.md

File metadata and controls

84 lines (60 loc) · 3.42 KB
Raw

Shambolic Banana Barbel

Medium

getSkippedBitmap() can read out of bounds memory

Summary

Calling getSkippedBitmap() on a batch header using an out of bounds index doesn't perform any validations on the index, and allows us to read into arbitrary memory.

Root Cause

In BatchHeaderCodecV0.sol, the getSkippedBitmap() function is used to access a bitmap word at a specific index.

function getSkippedBitmap(uint256 batchPtr, uint256 index) internal pure returns (uint256 _bitmap) {
    assembly {
        batchPtr := add(batchPtr, BATCH_HEADER_FIXED_LENGTH)
        _bitmap := mload(add(batchPtr, mul(index, 32)))
    }
}

The function does not check if the index is within the bounds of the bitmap, which can lead to out-of-bounds memory access.

Internal Preconditions

  1. getSkippedBitmap() must be able to be called with an index that is out of bounds.

External Preconditions

None

Attack Path

N/A

Impact

getSkippedBitmap() is able to return an invalid bitmap if it is called with an index that is out of bounds. For example, if we reached outside of the memory that has been used so far, we will get the 0 bitmap, which will imply that no messages were skipped.

PoC

The following test can be added to BatchHeaderCodecV0.t.sol to demonstrate the issue:

function testPapa_getSkippedBitmap() public {
    bytes memory batchHeader0 = new bytes(249 + 32);
    assembly {
        mstore(add(batchHeader0, 0x20), shl(248, 1)) // version
        mstore(add(batchHeader0, add(0x20, 1)), shl(192, 1)) // batchIndex = 1
        mstore(add(batchHeader0, add(0x20, 9)), shl(192, 1)) // l1MessagePopped = 1
        mstore(add(batchHeader0, add(0x20, 17)), shl(192, 1)) // totalL1MessagePopped = 1
        mstore(add(batchHeader0, add(0x20, 25)), ZERO_VERSIONED_HASH) // l1dataHash
        mstore(add(batchHeader0, add(0x20, 57)), ZERO_VERSIONED_HASH) // l2 tx blob versioned hash
        mstore(add(batchHeader0, add(0x20, 89)), ZERO_VERSIONED_HASH) // prevStateHash
        mstore(add(batchHeader0, add(0x20, 121)), ZERO_VERSIONED_HASH) // postStateHash
        mstore(add(batchHeader0, add(0x20, 153)), ZERO_VERSIONED_HASH) // withdrawRootHash
        mstore(add(batchHeader0, add(0x20, 185)), ZERO_VERSIONED_HASH) // sequencerSetVerifyHash
        mstore(add(batchHeader0, add(0x20, 217)), ZERO_VERSIONED_HASH) // parentBatchHash
        mstore(add(batchHeader0, add(0x20, 249)), ZERO_VERSIONED_HASH) // bitmap0
    }

    // we can now stick extra values later in memory
    bytes memory x = abi.encodePacked(uint(1056));

    // access the pointer to the batch header
    uint batchPtr;
    assembly { batchPtr := add(batchHeader0, 0x20) }

    // calling getSkippedBitmap with index = 0 gets the correct bitmap
    assertEq(BatchHeaderCodecV0.getSkippedBitmap(batchPtr, 0), uint(ZERO_VERSIONED_HASH));

    // calling with an index greater than 1 reaches out of bounds
    // 1056 = 0x420
    assertEq(
        bytes32(BatchHeaderCodecV0.getSkippedBitmap(batchPtr, 3)),
        0x0000000000042000000000000000000000000000000000000000000000000000
    );
}

Mitigation

Since we've already validated the length of the skipped bitmap against getL1MessagePopped(), we can call this function with the batchPtr to access the bitmap length, and then check the index against it.