You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "GumpacG (via GitHub)" <gi...@apache.org> on 2026/08/04 22:09:04 UTC
[PR] TINKERPOP-3281 Reject malformed GraphBinary length prefixes [tinkerpop]
GumpacG opened a new pull request, #3610:
URL: https://github.com/apache/tinkerpop/pull/3610
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 demand gigabytes and OOM Gremlin Server or a connecting driver.
## What changed
- Validate each length/count against the bytes actually remaining before it sizes an allocation or bounds a loop;
reject negative or oversized with `IOException`.
- Cap eager pre-sizing of growable collections/maps (grow on demand instead).
- Reject truncated length prefixes and negative `BulkSet` bulk values.
- Covers `String`, `ByteBuffer`, `BigInteger`, `InetAddress`, list, set, map, `Tree`, `BulkSet`, `Bytecode`, `P`
and graph. Well-formed messages are unaffected.
## Testing
`GraphBinaryLengthPrefixTest` (rejection + large round-trip cases); full GraphBinary compatibility suite green.
<br>
See: https://issues.apache.org/jira/browse/TINKERPOP-3281
Assisted-by: Kiro: Claude Opus 4.8
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TINKERPOP-3281 Reject malformed GraphBinary length prefixes [tinkerpop]
Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu commented on PR #3610:
URL: https://github.com/apache/tinkerpop/pull/3610#issuecomment-5257421071
VOTE+1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TINKERPOP-3281 Reject malformed GraphBinary length prefixes [tinkerpop]
Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu commented on code in PR #3610:
URL: https://github.com/apache/tinkerpop/pull/3610#discussion_r3730688568
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/SimpleTypeSerializer.java:
##########
@@ -68,6 +68,39 @@ public T readValue(final Buffer buffer, final GraphBinaryReader context, final b
*/
protected abstract T readValue(final Buffer buffer, final GraphBinaryReader context) throws IOException;
+ /**
+ * Reads a length or element-count prefix and validates it before it is used to size an allocation. A negative
+ * value, or one larger than the number of bytes actually remaining in the buffer, cannot be legitimate since
+ * every counted element or byte needs at least one byte on the wire, so it is rejected rather than allowed to
+ * drive a large allocation from a small message.
+ */
+ protected static int readSizePrefix(final Buffer buffer) throws IOException {
+ if (buffer.readableBytes() < Integer.BYTES)
+ throw new IOException(String.format(
Review Comment:
We should probably throw SerializationException from serializers as that is already a type of IOException
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TINKERPOP-3281 Reject malformed GraphBinary length prefixes [tinkerpop]
Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu commented on PR #3610:
URL: https://github.com/apache/tinkerpop/pull/3610#issuecomment-5257569041
Merging as lazy consensus reached.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TINKERPOP-3281 Reject malformed GraphBinary length prefixes [tinkerpop]
Posted by "GumpacG (via GitHub)" <gi...@apache.org>.
GumpacG commented on code in PR #3610:
URL: https://github.com/apache/tinkerpop/pull/3610#discussion_r3760175772
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/SimpleTypeSerializer.java:
##########
@@ -68,6 +68,39 @@ public T readValue(final Buffer buffer, final GraphBinaryReader context, final b
*/
protected abstract T readValue(final Buffer buffer, final GraphBinaryReader context) throws IOException;
+ /**
+ * Reads a length or element-count prefix and validates it before it is used to size an allocation. A negative
+ * value, or one larger than the number of bytes actually remaining in the buffer, cannot be legitimate since
+ * every counted element or byte needs at least one byte on the wire, so it is rejected rather than allowed to
+ * drive a large allocation from a small message.
+ */
+ protected static int readSizePrefix(final Buffer buffer) throws IOException {
+ if (buffer.readableBytes() < Integer.BYTES)
+ throw new IOException(String.format(
Review Comment:
SerializationException lives in gremlin-util which depends on gremlin-core, not the other way around. To do this, SerializationException would have to be duplicated, which is not recommended. Won't do as SerializationException is not in the right place.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TINKERPOP-3281 Reject malformed GraphBinary length prefixes [tinkerpop]
Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu merged PR #3610:
URL: https://github.com/apache/tinkerpop/pull/3610
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TINKERPOP-3281 Reject malformed GraphBinary length prefixes [tinkerpop]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3610:
URL: https://github.com/apache/tinkerpop/pull/3610#issuecomment-5185279365
## [Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/3610?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 77.08%. Comparing base ([`4722890`](https://app.codecov.io/gh/apache/tinkerpop/commit/4722890ed831ba4f71c3916b5e6eafcf85439345?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`bdb4220`](https://app.codecov.io/gh/apache/tinkerpop/commit/bdb4220156ad04569f35484997172a846de6d8ff?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 69 commits behind head on 3.7-dev.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 3.7-dev #3610 +/- ##
=============================================
+ Coverage 75.49% 77.08% +1.58%
=============================================
Files 1092 28 -1064
Lines 67208 5337 -61871
Branches 7391 0 -7391
=============================================
- Hits 50742 4114 -46628
+ Misses 13837 1020 -12817
+ Partials 2629 203 -2426
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/tinkerpop/pull/3610?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org