TINKERPOP-3281: Reject malformed GraphBinary length prefixes (master)

[Iryna-Kahamlyk]

JIRA: https://issues.apache.org/jira/browse/TINKERPOP-3281

GraphBinary sized heap allocations from an unvalidated 4-byte length prefix before reading the payload, so a tiny pre-auth frame (~6–21 bytes) could request gigabytes of memory and OOM Gremlin Server or a connecting driver.

Changes

This was already fixed on 3.7-dev in #3610. However, the same fix does not work on master. A shared readSizePrefix() in SimpleTypeSerializer that unconditionally calls buffer.readableBytes() would break the active streaming driver path. Instead, validation is split based on how the prefix is used, with readableBytes() checked only when the buffer can report it.

The following helpers were added:

• readByteLength() — fixed-size byte allocations (String, BigInteger, Binary/ByteBuffer); rejects negative values, enforces an explicit cap, and checks readableBytes() when known.
• readElementCount() — growable containers (List/Set/Map/Tree/Graph); rejects negative values and checks readableBytes() when known, with no explicit cap.
• readFixedArrayCount() — fixed arrays (P arguments); validated like byte lengths since the array cannot grow incrementally.
• cappedInitialCapacity() — caps speculative ArrayList/HashMap pre-sizing.

These are applied across StringSerializer, BigIntegerSerializer, BinarySerializer, CollectionSerializer(List/Set), MapSerializer, TreeSerializer, GraphSerializer, ListSerializer, and PSerializer.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 92.92035% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.81%. Comparing base (a28cd1f) to head (f4d0037).
⚠️ Report is 733 commits behind head on master.

Files with missing lines	Patch %	Lines
...lin/structure/io/binary/GraphBinaryReadLimits.java	81.81%	1 Missing and 3 partials ⚠️
...mlin/structure/io/binary/types/ListSerializer.java	50.00%	1 Missing and 1 partial ⚠️
...gremlin/structure/io/binary/types/PSerializer.java	0.00%	1 Missing ⚠️
...tructure/io/binary/types/SimpleTypeSerializer.java	96.42%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3703      +/-   ##
============================================
+ Coverage     76.35%   76.81%   +0.45%     
- Complexity    13424    14402     +978     
============================================
  Files          1012     1039      +27     
  Lines         60341    64913    +4572     
  Branches       7075     7723     +648     
============================================
+ Hits          46076    49861    +3785     
- Misses        11548    11965     +417     
- Partials       2717     3087     +370     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Cole-Greer]

If I understand this correctly, this imposes a hard cap of the size of single object being deserialized. Attempts to deserialize a single String in excess of 64MB would result in a SerializationException. Is this correct?

If so, I think we need to make these limits configurable in both the server and the driver. 64MB seems like a completely reasonable limit, and I would argue it's bad practice to store something that large as a property of something, but I'm sure there's someone storing a novel as a vertex property who will need a workaround for this.

The same applies for the following caps as well. We need to ensure a viable success path for users with excessively large data.