[
https://issues.apache.org/jira/browse/TINKERPOP-3281?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18127251#comment-18127251
]
ASF GitHub Bot commented on TINKERPOP-3281:
-------------------------------------------
Cole-Greer commented on code in PR #3703:
URL: https://github.com/apache/tinkerpop/pull/3703#discussion_r4275910095
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/SimpleTypeSerializer.java:
##########
@@ -35,6 +35,23 @@
public abstract class SimpleTypeSerializer<T> implements TypeSerializer<T> {
private final DataType dataType;
+ /**
+ * Upper bound, in bytes, on a length prefix sizing a fixed byte
allocation ({@code byte[]}, {@code ByteBuffer}),
+ * enforced even when {@link Buffer#hasKnownRemainingLength()} is {@code
false}.
+ */
+ private static final int MAX_BYTE_LENGTH = 64 * 1024 * 1024;
Review Comment:
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.
> GraphBinary deserializer unbounded pre-allocation from length prefixes
> ----------------------------------------------------------------------
>
> Key: TINKERPOP-3281
> URL: https://issues.apache.org/jira/browse/TINKERPOP-3281
> Project: TinkerPop
> Issue Type: Bug
> Components: io
> Affects Versions: 4.0.0, 3.7.7, 3.8.2
> Reporter: Guian Gumpac
> Priority: Major
> Fix For: 3.7.7, 3.8.2
>
>
> GraphBinary value types size a heap allocation (or bound a read loop) from a
> 4-byte length/count prefix before the payload is read or checked against the
> bytes actually remaining. Because GraphBinary is deserialized
> pre-authentication on the default wire path, a tiny malformed frame (~6-21
> bytes) can declare a multi-hundred-megabyte to multi-gigabyte allocation and
> drive Gremlin Server to OutOfMemoryError. maxContentLength bounds the frame,
> not a single declared length, and the binary decoder catches only
> SerializationException, so the OOM escapes. The surface is symmetric: a
> malicious or on-path server can OOM a connecting driver the same way. The
> same shape recurs across String, Collection (list/set), Map, BigInteger,
> InetAddress, ByteBuffer, Bytecode, P, Tree, graph and BulkSet.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
