Unvalidated Memory Access in `readMem` and `writeMem` Functions

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-07-optimism/blob/main/packages/contracts-bedrock/src/cannon/MIPS.sol#L538

Vulnerability details

Impact

The readMem() and writeMem() functions do not properly validate the memory address being accessed. This could lead to unauthorized access to memory outside the intended ranges, which could then allow a black hat to read or modify sensitive data, in creative ways. As the contract is using a 32-bit address space uint32 for addresses without implementing proper bounds checking, possibly then allowing access to the entire 4GB address space.

Specific attack vectors:

• Overwriting the preimageKey and/or preimageOffset to gain unauthorized access to preimage data.
• Manipulating the pc or nextPC values to alter program flows, even possible paths to bypass security checks.
• Adjusting with intent the heap value to cause memory allocation issues.
• Altering the exitCode or exited flags to prematurely terminate or continue execution inappropriately.

Proof of Concept

The bug leading to the attack vectors mentioned, is present in both readMem() and writeMem() functions but here is the relevant snippet of readMem():

function readMem(uint32 _addr, uint8 _proofIndex) internal pure returns (uint32 out_) {
    unchecked {
        uint256 offset = proofOffset(_proofIndex);

        assembly {
            // Validate the address alignment.
            if and(_addr, 3) { revert(0, 0) }

            // Load the leaf value.
            let leaf := calldataload(offset)
            offset := add(offset, 32)

            // ... (merkle proof verification)

            // Bits to shift = (32 - 4 - (addr % 32)) * 8
            let shamt := shl(3, sub(sub(32, 4), and(_addr, 31)))
            out_ := and(shr(shamt, leaf), 0xFFffFFff)
        }
    }
}

The function only checks for 4-byte alignment but does not validate if the address is within the allowed memory range.

Add the below foundry test function to MIPS.t.sol to show the bug, will need to fix setup when running if bugs occur:

    function testUnauthorizedMemoryAccess() public {
        // Setup initial state
        bytes32 initialMemRoot = bytes32(uint256(1));
        uint32 initialPC = 0x1000;
        uint32 initialHeap = 0x40000000;

        bytes memory stateData = abi.encodePacked(
            initialMemRoot,    // memRoot
            bytes32(0),        // preimageKey
            uint32(0),         // preimageOffset
            initialPC,         // pc
            initialPC + 4,     // nextPC
            uint32(0),         // lo
            uint32(0),         // hi
            initialHeap,       // heap
            uint8(0),          // exitCode
            false,             // exited
            uint64(0),         // step
            uint32(0x8C020000) // lw $v0, 0($zero) - load word instruction
        );

        // Add register data (32 registers)
        for (uint i = 0; i < 32; i++) {
            stateData = abi.encodePacked(stateData, uint32(0));
        }

        // Create a proof that allows reading from any address
        bytes memory proof = new bytes(28 * 32);
        for (uint i = 0; i < 28; i++) {
            bytes32 proofElement = bytes32(uint256(i + 1));
            assembly {
                mstore(add(proof, mul(add(i, 1), 32)), proofElement)
            }
        }

        // Step 1: Read from a very high address (to out of bounds)
        uint32 highAddress = 0xFFFFFFFC;
        bytes memory maliciousStateData = abi.encodePacked(
            stateData,
            uint32(0x8C020000 | highAddress) // lw $v0, 0($zero) with high address
        );

        vm.expectRevert("Memory address out of bounds");
        mips.step(maliciousStateData, proof, bytes32(0));

        // Step 2: Write to a very high address (should be out of bounds)
        maliciousStateData = abi.encodePacked(
            stateData,
            uint32(0xAC020000 | highAddress) // sw $v0, 0($zero) with high address
        );

        vm.expectRevert("Memory address out of bounds");
        mips.step(maliciousStateData, proof, bytes32(0));
    }

I did this PoC to:

• 1. Attempt to read from a very high memory address (0xFFFFFFFC).
• 2. Attempt to write to the same high memory address.

Tools Used

Manual review, VsCode, Foundry, AuditWizard

Recommended Mitigation Steps

1. Add these strict bounds checking statements in readMem() and writeMem():

uint32 constant MAX_MEMORY_ADDRESS = 0x7FFFFFFF; // Define this better to prevent

function readMem(uint32 _addr, uint8 _proofIndex) internal pure returns (uint32 out_) {
    unchecked {
        require(_addr <= MAX_MEMORY_ADDRESS, "Memory address out of bounds");
        // rest is same
    }
}

function writeMem(uint32 _addr, uint8 _proofIndex, uint32 _val) internal pure {
    unchecked {
        require(_addr <= MAX_MEMORY_ADDRESS, "Memory address out of bounds");
        // rest is same
    }
}

• Make sure carefully choose the value for MAX_MEMORY_ADDRESS based on the specific MIPS implementation as of current and memory layout used in the contract. The constant should be set to limit the accessible memory range to only what is actually necessary, bounded specifically, to prevent gaps that can be targeted by creative black hats.

Assessed type

Context

[mbaxter]

There is no sensitive data to read or write - the contracts are stateless. An attacker also has no control over the program running in the VM - that is defined by the absolute prestate. The attack vectors mentioned all require manipulating fields in the state, but this is the same as posting an invalid claim. An honest actor will just counter the invalid claim.

[c4-judge]

zobront marked the issue as unsatisfactory: Invalid

[zobront]

@CrystallineButterfly After further discussion with the sponsor, I believe this issue fits into the same category of others that have been accepted:

• Potential issue in the MIPS implementation
• No known current way to exploit it
• If it was exploitable, it would be extremely bad
• No guarantees that it can't be exploited

While this doesn't point to a specific deviation from the MIPS spec, it does fit the same criteria as above. For that reason, I will be upgrading the issue back to Medium (as the others of this type are) and including it in the final report.

[c4-judge]

zobront marked the issue as satisfactory

[c4-judge]

zobront marked the issue as selected for report