code-423n4 / 2024-07-optimism-findings

3 stars 0 forks source link

Uncontrolled memory growth in `handleSyscall()` #87

Closed howlbot-integration[bot] closed 3 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

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

Vulnerability details

Impact

The handleSyscall() function contains a vulnerability in the current implementation of the mmap syscall (syscall number 4090). This function allows for uncontrolled memory allocation, with an attack vector leading to potential DOS or excessive gas consumption.

Steps to Exploit:

It can lead to significant resource waste, causing transactions to fail and thereby potentially disrupting the system's overall functionality.

Proof of Concept

Vulnerable code here, from handleSyscall():

if (syscall_no == 4090) {
    uint32 sz = a1;
    if (sz & 4095 != 0) {
        // adjust size to align with page size
        sz += 4096 - (sz & 4095);
    }
    if (a0 == 0) {
        v0 = state.heap;
        state.heap += sz;
    } else {
        v0 = a0;
    }
}

Above code allows for arbitrary memory allocation without upper bound checks. This enables an attacker being able to exploit this, by repeatedly calling the mmap syscall with large sizes, causing the state.heap to grow indefinitely and way too much.

Foundry PoC that should pass on your end, proving this vulnerability:

    function testMemoryGrowth() public {
        // Create a state with syscall number 4090 (mmap)
        bytes memory stateData = abi.encodePacked(
            bytes32(0), // memRoot
            bytes32(0), // preimageKey
            uint32(0),  // preimageOffset
            uint32(0),  // pc
            uint32(4),  // nextPC
            uint32(0),  // lo
            uint32(0),  // hi
            uint32(0),  // heap
            uint8(0),   // exitCode
            false,      // exited
            uint64(0),  // step
            uint32(4090), // syscall number in $v0 (register 2)
            uint32(0),    // $a0 (register 4)
            uint32(1 << 30), // $a1 (register 5) - request 1GB of memory
            [uint32(0), uint32(0), uint32(0), uint32(0), // registers 0-3
             uint32(0), uint32(0), uint32(0), uint32(0), // registers 6-9
             uint32(0), uint32(0), uint32(0), uint32(0),
             uint32(0), uint32(0), uint32(0), uint32(0),
             uint32(0), uint32(0), uint32(0), uint32(0),
             uint32(0), uint32(0), uint32(0), uint32(0),
             uint32(0), uint32(0), uint32(0), uint32(0)]
        );

        bytes memory proof = new bytes(28 * 32); // Empty proof

        // Call step function multiple times
        for (uint i = 0; i < 5; i++) {
            bytes32 result = mips.step(stateData, proof, bytes32(0));

            // Decode the result to get the new state
            (,,,,,,,uint32 newHeap,,,,) = abi.decode(abi.encodePacked(result), (bytes32,bytes32,uint32,uint32,uint32,uint32,uint32,uint32,uint8,bool,uint64,uint32[32]));

            console.log("Heap size after iteration", i, ":", newHeap);

            // Update the state for the next iteration
            stateData = abi.encodePacked(result);
        }
    }
}

Tools Used

Manual review, Vscode & Foundry for PoC, AuditWizard

Recommended Mitigation Steps

Add an upper bound for the total heap size, and add checks to prevent excessive memory allocation in a single mmap call. And add a mechanism to track and limit the total memory allocated across multiple mmap calls.

Fix:

uint32 constant MAX_HEAP_SIZE = 1 << 30; // 1GB max heap size

if (syscall_no == 4090) {
    uint32 sz = a1;
    if (sz & 4095 != 0) {
        // adjust size to align with page size
        sz += 4096 - (sz & 4095);
    }
    if (a0 == 0) {
        require(state.heap + sz <= MAX_HEAP_SIZE, "Heap size limit exceeded");
        v0 = state.heap;
        state.heap += sz;
    } else {
        v0 = a0;
    }
}

Assessed type

DoS

ajsutton commented 4 months ago

This is invalid because the attacker doesn't control the code executed by the VM and is therefore unable to call mmap with arbitrary values. The program being executed is part of the ABSOLUTE_PRESTATE of the fault dispute game which is op-program compiled for MIPS in the current game configuration.

c4-judge commented 3 months ago

zobront marked the issue as unsatisfactory: Invalid