041.md

Nice Laurel Turtle

Medium

Incorrect Handling of publicInputHash in verify Function Causes Data Misalignment and Verification Failures

Summary

The ZkEvmVerifierV1.sol smart contract improperly handles the publicInputHash in its assembly block during the verification process, causing data misalignment and corruption of the proof structure.

Vulnerability Detail

The verify function of the ZkEvmVerifierV1.sol contract is responsible for verifying aggregate zero-knowledge (ZK) proofs by interacting with an external PLONK verifier contract.

function verify(bytes calldata aggrProof, bytes32 publicInputHash) external view override {
    address _verifier = PLONK_VERIFIER;
    bool success;

    assembly {
        let p := mload(0x40)
        calldatacopy(p, aggrProof.offset, 0x180)
        for {
            let i := 0
        } lt(i, 0x400) {
            i := add(i, 0x20)
        } {
            mstore(add(p, sub(0x560, i)), and(publicInputHash, 0xff))
            publicInputHash := shr(8, publicInputHash)
        }
        calldatacopy(add(p, 0x580), add(aggrProof.offset, 0x180), sub(aggrProof.length, 0x180))

        success := staticcall(gas(), _verifier, p, add(aggrProof.length, 0x400), 0x00, 0x00)
    }

    if (!success) {
        revert VerificationFailed();
    }
}

The vulnerability stems from the incorrect handling of publicInputHash within the assembly block:

In this, the loop iterates 32 times (0x400 / 0x20 = 32), processing each byte of publicInputHash. It uses mstore to store each byte, which incorrectly writes 32 bytes (256 bits) for each byte of publicInputHash

Clarifying the Loop Calculation

for {
    let i := 0
} lt(i, 0x400) {
    i := add(i, 0x20)
} {
    mstore(add(p, sub(0x560, i)), and(publicInputHash, 0xff))
    publicInputHash := shr(8, publicInputHash)
}

Here’s the information presented:

• Loop Condition Breakdown:

  • 0x400 (Hexadecimal): Equals 1024 in decimal.
  • 0x20 (Hexadecimal): Equals 32 in decimal.
  • Loop Iterations: 1024 / 32 = 32, so the loop iterates 32 times.

• Intended Behavior:

  • Purpose: To process and store each byte of the publicInputHash.
  • Approach: Extract each byte from publicInputHash and store it sequentially in memory.
  • Expected Storage: 32 bytes (since publicInputHash is a bytes32 type).

• Actual Behavior Due to Vulnerability:

  • Issue: The loop uses mstore to store each single byte.
  • mstore Functionality: Stores 32 bytes (a full word) at a time.

• Result:

  • Per Iteration: Storing one byte using mstore writes 32 bytes, introducing 31 bytes of unintended padding.
  • Total Storage: 32 iterations * 32 bytes = 1024 bytes instead of the intended 32 bytes.

Impact

1. Memory Overflow: Each byte of publicInputHash is incorrectly stored as a 32-byte word, causing a memory overflow from 32 bytes to 1024 bytes.
2. Invalid Proof Structure: The PLONK verifier receives malformed proof data, causing verification failures for legitimate proofs.
3. Excessive Gas Consumption: Storing 1024 bytes instead of 32 bytes significantly increases gas costs for each verification attempt. Inefficient memory usage results in unnecessarily high gas costs.

Code Snippet

https://github.com/sherlock-audit/2024-08-morphl2/blob/main/morph/contracts/contracts/libraries/verifier/ZkEvmVerifierV1.sol#L48-L55

Tool used

Manual Review

Recommendation

• Replace mstore with mstore8 to store each byte individually, preventing unnecessary padding, mstore8 stores a single byte at a specified memory location, ensuring that only the intended byte is written without additional padding.

assembly {
      let p := mload(0x40)
      calldatacopy(p, aggrProof.offset, 0x180)
      for { let i := 0 } lt(i, 0x20) { i := add(i, 1) } {
          mstore8(add(p, 0x180 + i), byte(i, publicInputHash))
      }
      calldatacopy(add(p, 0x200), add(aggrProof.offset, 0x180), sub(aggrProof.length, 0x180))
  
      success := staticcall(gas(), _verifier, p, add(aggrProof.length, 0x20), 0x00, 0x00)
  }

• Adjust the loop to iterate exactly 32 times, corresponding to the 32 bytes of publicInputHash. By iterating 32 times and using mstore8, each byte of publicInputHash is correctly stored without excess memory allocation.

  for { let i := 0 } lt(i, 0x20) { i := add(i, 1) } {
      mstore8(add(p, 0x180 + i), byte(i, publicInputHash))
  }