Skip to content

fix: validate manifest offset bounds before allocation in Deserialization - #364

Merged
max-leuthaeuser merged 1 commit into
masterfrom
max/check-manifest-size
Jul 27, 2026
Merged

fix: validate manifest offset bounds before allocation in Deserialization#364
max-leuthaeuser merged 1 commit into
masterfrom
max/check-manifest-size

Conversation

@max-leuthaeuser

@max-leuthaeuser max-leuthaeuser commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

QT-1269 - Root Cause

When loading a flatgraph file, readManifest reads an 8-byte little-endian manifest offset from the file header, then computes:

val manifestSize = channel.size() - manifestOffset

If the file is truncated (e.g. interrupted download), the manifest offset stored in the header points beyond the actual end of the file, making manifestSize a negative Long. Two existing guards both failed to catch this:

  1. manifestSize > fileSize — only triggers if manifestOffset < 0, which is not the case for a truncated file
  2. manifestSize > Int.MaxValue — a negative Long is never greater than Int.MaxValue

The negative Long then passes both checks, .toInt narrows it to a negative Int, and ByteBuffer.allocate(negativeInt) throws:

java.lang.IllegalArgumentException: capacity < 0: (-237638205 < 0)
    at java.base/java.nio.ByteBuffer.allocate(ByteBuffer.java:390)
    at flatgraph.storage.Deserialization$.readManifest(Deserialization.scala:202)

Fix

Replace the flawed manifestSize > fileSize guard with an explicit bounds check on manifestOffset itself, before manifestSize is even computed:

if (manifestOffset < 0 || manifestOffset >= channel.size())
  throw new DeserializationException(
    s"corrupt file: manifest offset ($manifestOffset) is out of bounds (file size: ${channel.size()})"
  )

This produces a clear, actionable error message instead of a cryptic JVM crash deep in buffer allocation.

Verification

Confirmed against the real cpg.bin.zip (121.5 MB on disk, manifest offset pointing to 348.2 MB):

Magic bytes:        FLT GRPH   ← valid header
File size:          127,451,136 bytes (121.5 MB)
Manifest offset:    365,089,341 bytes (348.2 MB)
Manifest size:      -237,638,205 bytes  ← was passed to ByteBuffer.allocate
Offset out of bounds: True

Fixes https://harness.atlassian.net/browse/QT-1269

@max-leuthaeuser
max-leuthaeuser requested a review from bbrehm July 27, 2026 11:25
@max-leuthaeuser
max-leuthaeuser force-pushed the max/check-manifest-size branch 2 times, most recently from 0384b1b to b62cea2 Compare July 27, 2026 11:35
@max-leuthaeuser
max-leuthaeuser force-pushed the max/check-manifest-size branch from b62cea2 to 8776397 Compare July 27, 2026 11:37
@max-leuthaeuser
max-leuthaeuser merged commit 6068a4c into master Jul 27, 2026
1 check passed
@max-leuthaeuser
max-leuthaeuser deleted the max/check-manifest-size branch July 27, 2026 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants