From 76f5a36adef499dd689adcc6fa62455bd48e32a8 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Fri, 31 Jul 2026 13:40:48 -0700 Subject: [PATCH 1/2] Restrict GraphSON 1.0 embedded-type deserialization Assisted-by: Kiro: Claude Opus 4.8 --- CHANGELOG.asciidoc | 1 + THREAT_MODEL.md | 5 +- docs/src/dev/io/graphson.asciidoc | 6 +- docs/src/upgrade/release-3.7.x.asciidoc | 31 ++++++ .../structure/io/graphson/GraphSONMapper.java | 79 ++++++++++++++- .../example/gadget/GraphSONTestGadgets.java | 67 +++++++++++++ ...GraphSONMapperPartialEmbeddedTypeTest.java | 95 +++++++++++++++++++ 7 files changed, 279 insertions(+), 5 deletions(-) create mode 100644 gremlin-core/src/test/java/com/example/gadget/GraphSONTestGadgets.java diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index ef4250fa9f5..77bf6bc8d3f 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima === TinkerPop 3.7.7 (Release Date: NOT OFFICIALLY RELEASED YET) * Disabled unsafe Java deserialization on the Gryo IO paths and added `GryoMapper.Builder.javaSerializationAllowed(boolean)` to control it. +* Hardened GraphSON 1.0 typed deserialization so that reading untrusted input can no longer construct arbitrary Java classes. * Fixed `subgraph()` to throw a descriptive error identifying the required `Edge` input instead of an internal `ClassCastException` when the traversal produces a non-edge value. * Fixed `where(P)` to throw a descriptive error identifying the required String scope key (and suggesting `is(P)` for value comparisons) instead of an internal `ClassCastException` when given a non-String predicate value. * Fixed `PeerPressure.property_name` in `gremlin-python` incorrectly mapping to the `pageRank` property name token. diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md index 1b2f80bc4f6..8576dbdf099 100644 --- a/THREAT_MODEL.md +++ b/THREAT_MODEL.md @@ -232,7 +232,7 @@ Knobs that change which security properties hold (Gremlin Server, `gremlin-serve `gremlin-server-secure.yaml`. The docs are explicit that TinkerPop offers no complete out-of-the-box protection against nefarious scripts. *(documented — `gremlin-applications.asciidoc` "Protecting Script Execution", the two sample configs)* -- **Enabled serializers** — wire set is GraphSON 3.0 + GraphBinary. Gryo is IO-format-only, not on the wire, +- **Enabled serializers** — the default wire set is GraphSON 3.0 + GraphBinary. The typed GraphSON 1.0 serializer (`vnd.gremlin-v1.0+json`) is also shippable and constrains its `@class` default typing to an allow-list (§8). Gryo is IO-format-only, not on the wire, and defaults to a locked registration allow-list (`registrationRequired=true`). Disabling that lock removes the untrusted-input protection (§9). *(documented — sample configs, `gremlin-applications.asciidoc` "Serialization")* @@ -341,7 +341,8 @@ Per-surface trust table: IO paths build** (`registrationRequired=true` plus `javaSerializationAllowed=false`, i.e. `io()`, `GryoReader`, `GryoWriter`, `GryoIo`, and the Hadoop Gryo input/output formats) reading attacker bytes do not reach native Java deserialization - (`ObjectInputStream.readObject()`). Because `inject()` and value arguments let a request carry any + (`ObjectInputStream.readObject()`), and do not instantiate an arbitrary class named in the document: GraphSON 1.0 + embedded types constrain Jackson default typing to a validated type allow-list. Because `inject()` and value arguments let a request carry any supported type, a bug in a **registered** type's (de)serializer that crashes/OOMs the reader is also in-model, on **both** the server (request) and the GLV (response) side. The GraphML reader disables external entities and DTDs by default (XXE-safe). *Violation symptom:* deserialization gadget / RCE / XXE, diff --git a/docs/src/dev/io/graphson.asciidoc b/docs/src/dev/io/graphson.asciidoc index 89d6720d7c8..eb4bb37919b 100644 --- a/docs/src/dev/io/graphson.asciidoc +++ b/docs/src/dev/io/graphson.asciidoc @@ -112,7 +112,11 @@ Version 1.0 of GraphSON was released with TinkerPop 3.0.0. It is referred to by When types are embedded, GraphSON uses the standard link:https://github.com/FasterXML/jackson-databind[Jackson] type embedding approach that writes the full Java class name into a "@class" field in the JSON. While this approach isn't especially language agnostic it does at least give -some hint as to what the expected type is. +some hint as to what the expected type is. On read, that class name is constrained to a validated allow-list so that +an untrusted document cannot name an arbitrary class for construction. A class outside the allowed set is refused. +The allow-list permits the `java.lang`, `java.util`, `java.math`, `java.time`, `java.sql` and `org.apache.tinkerpop` +packages plus array types. A provider or application reading trusted input can add its own package with +`GraphSONMapper.Builder.addAllowedTypeIdPrefix(String...)`, for example `addAllowedTypeIdPrefix("com.example.")`. This section focuses on non-embedded types and their formats as there was little usage of embedded types in generalized object serialization use cases. The format was simply too cumbersome to parse of non-Jackson enabled libraries and the diff --git a/docs/src/upgrade/release-3.7.x.asciidoc b/docs/src/upgrade/release-3.7.x.asciidoc index 4b8440eab03..e721fe5fcb5 100644 --- a/docs/src/upgrade/release-3.7.x.asciidoc +++ b/docs/src/upgrade/release-3.7.x.asciidoc @@ -76,6 +76,37 @@ GryoIo.build().graph(graph).onMapper(m -> ((GryoMapper.Builder) m).javaSerializa See: link:https://issues.apache.org/jira/browse/TINKERPOP-3278[TINKERPOP-3278] +==== GraphSON 1.0 Restricts Embedded-Type Deserialization + +GraphSON 1.0 with embedded types (`TypeInfo.PARTIAL_TYPES`) reconstructs a value from the Java class named in a +`@class` property using Jackson default typing. That mechanism previously resolved any class named in the document, +which is an unsafe-deserialization vector when reading untrusted input. Default typing is now constrained by a +`PolymorphicTypeValidator` that permits only the value packages GraphSON 1.0 legitimately round-trips (`java.lang`, +`java.util`, `java.math`, `java.time`, `java.sql`, `org.apache.tinkerpop`) plus array types. Standard scalar, collection and +array values are unaffected. + +A document whose `@class` names a type outside that set is now refused rather than instantiated: + +[source,text] +---- +Could not resolve subtype ...: Configured `PolymorphicTypeValidator` ... denied resolution +---- + +Applications that deliberately read a custom or provider value type through GraphSON 1.0 embedded types (for +example a type registered via an `IoRegistry`) can re-enable it, for trusted input, by trusting its package on the +mapper: + +[source,java] +---- +GraphSONMapper.build().version(GraphSONVersion.V1_0).typeInfo(TypeInfo.PARTIAL_TYPES) + .addAllowedTypeIdPrefix("com.example.").create(); +---- + +GraphSON 2.0 and 3.0 were never affected: they resolve types through a fixed registry rather than by arbitrary +class name. + +See: link:https://issues.apache.org/jira/browse/TINKERPOP-3279[TINKERPOP-3279] + ==== conjoin() Step Null Handling The `conjoin()` step previously returned `null` when elements in the incoming list are `null`. This behavior has diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapper.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapper.java index cda357caaeb..053044ab72e 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapper.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapper.java @@ -27,6 +27,9 @@ import org.apache.tinkerpop.shaded.jackson.core.StreamReadConstraints; import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper; import org.apache.tinkerpop.shaded.jackson.databind.SerializationFeature; +import org.apache.tinkerpop.shaded.jackson.databind.JavaType; +import org.apache.tinkerpop.shaded.jackson.databind.cfg.MapperConfig; +import org.apache.tinkerpop.shaded.jackson.databind.jsontype.PolymorphicTypeValidator; import org.apache.tinkerpop.shaded.jackson.databind.jsontype.TypeResolverBuilder; import org.apache.tinkerpop.shaded.jackson.databind.jsontype.impl.StdTypeResolverBuilder; import org.apache.tinkerpop.shaded.jackson.databind.module.SimpleModule; @@ -63,6 +66,12 @@ * @author Stephen Mallette (http://stephen.genoprime.com) */ public class GraphSONMapper implements Mapper { + + // GraphSON 1.0 embedded types reconstruct a value from the class named in a "@class" property via Jackson + // default typing; constrain it to safe packages so an untrusted payload cannot name an arbitrary class. + // Providers can extend this with Builder.addAllowedTypeIdPrefix(...) for trusted input. + private static final List GRAPHSON_1_0_DEFAULT_TYPE_PREFIXES = Arrays.asList( + "java.lang.", "java.util.", "java.math.", "java.time.", "java.sql.", "org.apache.tinkerpop."); public static final int DEFAULT_MAX_NUMBER_LENGTH = 10000; private final List customModules; @@ -71,6 +80,7 @@ public class GraphSONMapper implements Mapper { private final GraphSONVersion version; private final TypeInfo typeInfo; private final StreamReadConstraints streamReadConstraints; + private final List allowedTypeIdPrefixes; private GraphSONMapper(final Builder builder) { this.customModules = builder.customModules; @@ -79,6 +89,7 @@ private GraphSONMapper(final Builder builder) { this.version = builder.version; this.streamReadConstraints = builder.streamReadConstraintsBuilder.build(); this.typeInfo = builder.typeInfo; + this.allowedTypeIdPrefixes = builder.allowedTypeIdPrefixes; } @Override @@ -128,8 +139,15 @@ public ObjectMapper createMapper() { om.setDefaultTyping(typer); } else if (version == GraphSONVersion.V1_0 || version == GraphSONVersion.V2_0) { if (typeInfo == TypeInfo.PARTIAL_TYPES) { - final TypeResolverBuilder typer = new StdTypeResolverBuilder() - .init(JsonTypeInfo.Id.CLASS, null) + final List allowedPrefixes = new ArrayList<>(GRAPHSON_1_0_DEFAULT_TYPE_PREFIXES); + allowedPrefixes.addAll(allowedTypeIdPrefixes); + final PolymorphicTypeValidator typeValidator = graphSON1dTypeValidator(allowedPrefixes); + final TypeResolverBuilder typer = new StdTypeResolverBuilder() { + @Override + public PolymorphicTypeValidator subTypeValidator(final MapperConfig config) { + return typeValidator; + } + }.init(JsonTypeInfo.Id.CLASS, null) .inclusion(JsonTypeInfo.As.PROPERTY) .typeProperty(GraphSONTokens.CLASS); om.setDefaultTyping(typer); @@ -152,6 +170,50 @@ public ObjectMapper createMapper() { return om; } + /** + * A {@link PolymorphicTypeValidator} for GraphSON 1.0 embedded types that decides purely from the type id name, + * so a disallowed class is refused before it is loaded (a name that only fails the check post-load would still + * have its static initializer run). A name is allowed when it, or an array's element type, starts with one of + * the trusted prefixes; primitive arrays are allowed. + */ + private static PolymorphicTypeValidator graphSON1dTypeValidator(final List allowedPrefixes) { + return new PolymorphicTypeValidator.Base() { + @Override + public Validity validateBaseType(final MapperConfig config, final JavaType baseType) { + return Validity.INDETERMINATE; + } + + @Override + public Validity validateSubClassName(final MapperConfig config, final JavaType baseType, + final String subClassName) { + return isAllowedTypeName(subClassName, allowedPrefixes) ? Validity.ALLOWED : Validity.DENIED; + } + + @Override + public Validity validateSubType(final MapperConfig config, final JavaType baseType, + final JavaType subType) { + return isAllowedTypeName(subType.getRawClass().getName(), allowedPrefixes) + ? Validity.ALLOWED : Validity.DENIED; + } + }; + } + + private static boolean isAllowedTypeName(final String typeName, final List allowedPrefixes) { + // unwrap array descriptors: "[Ljava.io.File;" -> "java.io.File", "[[B" -> primitive element + String name = typeName; + while (name.startsWith("[")) + name = name.substring(1); + if (name.length() <= 1) + return true; // primitive array element (e.g. [B, [I) carries no class to instantiate + if (name.startsWith("L") && name.endsWith(";")) + name = name.substring(1, name.length() - 1); + for (final String prefix : allowedPrefixes) { + if (name.startsWith(prefix)) + return true; + } + return false; + } + public GraphSONVersion getVersion() { return this.version; } @@ -204,6 +266,7 @@ public static class Builder implements Mapper.Builder { private StreamReadConstraints.Builder streamReadConstraintsBuilder = StreamReadConstraints.builder() .maxNumberLength(DEFAULT_MAX_NUMBER_LENGTH); private TypeInfo typeInfo = null; + private final List allowedTypeIdPrefixes = new ArrayList<>(); private Builder() { } @@ -288,6 +351,18 @@ public Builder typeInfo(final TypeInfo typeInfo) { return this; } + /** + * Adds a class-name prefix that GraphSON 1.0 embedded-type deserialization will accept in a {@code @class} + * property, in addition to the safe defaults ({@code java.lang.}, {@code java.util.}, {@code java.math.}, + * {@code java.time.}, {@code org.apache.tinkerpop.} and array types). Use this to re-enable a provider or + * application type read from trusted input; a graph document from an untrusted source should not be granted + * additional prefixes. Has no effect on GraphSON 2.0 or 3.0, which resolve types through a fixed registry. + */ + public Builder addAllowedTypeIdPrefix(final String... prefixes) { + this.allowedTypeIdPrefixes.addAll(Arrays.asList(prefixes)); + return this; + } + public Builder maxNumberLength(final int maxNumLength) { this.streamReadConstraintsBuilder.maxNumberLength(maxNumLength); return this; diff --git a/gremlin-core/src/test/java/com/example/gadget/GraphSONTestGadgets.java b/gremlin-core/src/test/java/com/example/gadget/GraphSONTestGadgets.java new file mode 100644 index 00000000000..90682aebba5 --- /dev/null +++ b/gremlin-core/src/test/java/com/example/gadget/GraphSONTestGadgets.java @@ -0,0 +1,67 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package com.example.gadget; + +import java.util.Objects; + +/** + * Test-only types in a package outside every allowlisted GraphSON 1.0 type prefix, used to exercise the + * GraphSON 1.0 embedded-type restriction and its {@code addAllowedTypeIdPrefix(...)} opt-out. + */ +public final class GraphSONTestGadgets { + + private GraphSONTestGadgets() { + } + + /** + * Records execution of its static initializer through a system property, so a test can observe whether the + * class was loaded/initialized without referencing it (which would itself trigger initialization). + */ + public static class StaticInitCanary { + public static final String FIRED_PROPERTY = "tinkerpop.test.graphson.staticInitCanary"; + static { + System.setProperty(FIRED_PROPERTY, "fired"); + } + public int x; + } + + /** + * A plain bean with no static-initializer side effect, used to verify the opt-out re-enables a trusted type. + */ + public static class SamplePojo { + public int x; + + public SamplePojo() { + } + + public SamplePojo(final int x) { + this.x = x; + } + + @Override + public boolean equals(final Object o) { + return o instanceof SamplePojo && ((SamplePojo) o).x == this.x; + } + + @Override + public int hashCode() { + return Objects.hashCode(x); + } + } +} diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperPartialEmbeddedTypeTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperPartialEmbeddedTypeTest.java index 523cdc95c86..4850921bea1 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperPartialEmbeddedTypeTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperPartialEmbeddedTypeTest.java @@ -24,7 +24,10 @@ import org.apache.tinkerpop.gremlin.process.traversal.TextP; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; import org.apache.tinkerpop.shaded.jackson.databind.JsonMappingException; +import com.example.gadget.GraphSONTestGadgets.SamplePojo; +import com.example.gadget.GraphSONTestGadgets.StaticInitCanary; import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper; +import org.apache.tinkerpop.shaded.jackson.databind.exc.InvalidTypeIdException; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -47,7 +50,9 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.IsInstanceOf.instanceOf; import static org.hamcrest.core.StringContains.containsString; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.fail; @@ -248,6 +253,96 @@ public void shouldHandleMapWithTypesUsingEmbedTypeSettingV1() throws Exception { assertEquals(100L, read.get("test")); } + @Test + public void shouldRejectDisallowedClassWithEmbedTypeSettingV1() { + // a "@class" naming a type outside the allowed value packages must be refused by the type validator + assertDeniedByTypeValidator(v1Typed(), + "{\"@class\":\"java.util.HashMap\",\"evil\":{\"@class\":\"java.io.File\",\"path\":\"/tmp/x\"}}"); + } + + @Test + public void shouldRoundTripArraysWithEmbedTypeSettingV1() throws Exception { + // arrays are legitimate GraphSON 1.0 values and rely on default typing; they must still round-trip + final ObjectMapper mapper = v1Typed(); + final Map m = new HashMap<>(); + m.put("strings", new String[]{"a", "b"}); + m.put("bytes", new byte[]{1, 2, 3}); + + final Map read = mapper.readValue(mapper.writeValueAsString(m), HashMap.class); + assertArrayEquals(new String[]{"a", "b"}, (String[]) read.get("strings")); + assertArrayEquals(new byte[]{1, 2, 3}, (byte[]) read.get("bytes")); + } + + @Test + public void shouldRoundTripSqlAndUtilValueTypesWithEmbedTypeSettingV1() throws Exception { + // common JDK value types in the allowed packages must round-trip + final ObjectMapper mapper = v1Typed(); + final Map m = new HashMap<>(); + m.put("ts", new java.sql.Timestamp(0L)); + m.put("uuid", new java.util.UUID(1L, 2L)); + m.put("big", new java.math.BigInteger("9")); + + final Map read = mapper.readValue(mapper.writeValueAsString(m), HashMap.class); + assertEquals(new java.sql.Timestamp(0L), read.get("ts")); + assertEquals(new java.util.UUID(1L, 2L), read.get("uuid")); + assertEquals(new java.math.BigInteger("9"), read.get("big")); + } + + @Test + public void shouldRejectArrayOfDisallowedComponentWithEmbedTypeSettingV1() { + // allowing arrays must not reopen the sink: an array whose component type is disallowed is still refused + assertDeniedByTypeValidator(v1Typed(), + "{\"@class\":\"java.util.HashMap\",\"evil\":[\"[Ljava.io.File;\",[\"/tmp/x\"]]}"); + } + + @Test + public void shouldRejectParameterizedDisallowedTypeWithEmbedTypeSettingV1() { + // a disallowed class named as a parameterized value type or key type must also be refused + assertDeniedByTypeValidator(v1Typed(), + "{\"@class\":\"java.util.HashMap\",\"k\":\"/tmp/x\"}"); + assertDeniedByTypeValidator(v1Typed(), + "{\"@class\":\"java.util.HashMap\",\"/tmp/x\":\"v\"}"); + } + + @Test + public void shouldAllowConfiguredTypeIdPrefixWithEmbedTypeSettingV1() throws Exception { + final String json = "{\"@class\":\"java.util.HashMap\",\"p\":{\"@class\":\"com.example.gadget.GraphSONTestGadgets$SamplePojo\",\"x\":42}}"; + + // denied by default (com.example is outside the safe set) + assertDeniedByTypeValidator(v1Typed(), json); + + // allowed once the package is explicitly trusted + final ObjectMapper mapper = GraphSONMapper.build().version(GraphSONVersion.V1_0).typeInfo(TypeInfo.PARTIAL_TYPES) + .addAllowedTypeIdPrefix("com.example.").create().createMapper(); + final Map read = mapper.readValue(json, HashMap.class); + assertEquals(new SamplePojo(42), read.get("p")); + } + + @Test + public void shouldNotLoadDisallowedClassWhenRefusingV1() { + // a refused @class must be denied by name before it is loaded, otherwise its static initializer runs + System.clearProperty(StaticInitCanary.FIRED_PROPERTY); + assertDeniedByTypeValidator(v1Typed(), + "{\"@class\":\"java.util.HashMap\",\"g\":{\"@class\":\"com.example.gadget.GraphSONTestGadgets$StaticInitCanary\",\"x\":1}}"); + assertNull("a refused @class must not be class-loaded (its static initializer must not run)", + System.getProperty(StaticInitCanary.FIRED_PROPERTY)); + } + + private static ObjectMapper v1Typed() { + return GraphSONMapper.build().version(GraphSONVersion.V1_0).typeInfo(TypeInfo.PARTIAL_TYPES).create().createMapper(); + } + + private static void assertDeniedByTypeValidator(final ObjectMapper mapper, final String json) { + try { + mapper.readValue(json, HashMap.class); + fail("deserialization of a disallowed @class must be refused"); + } catch (InvalidTypeIdException expected) { + // the polymorphic type validator refused to resolve the type id + } catch (Exception other) { + throw new AssertionError("expected InvalidTypeIdException, got " + other, other); + } + } + @Test public void shouldNotHandleMapWithTypesUsingEmbedTypeSettingV1() throws Exception { final ObjectMapper mapper = GraphSONMapper.build() From 3563220281827615dad30d0d2a10becab3bee164 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Wed, 5 Aug 2026 17:05:32 -0700 Subject: [PATCH 2/2] Made GraphSON 1 allowlist use GraphSON 2/3 registered types --- docs/src/dev/io/graphson.asciidoc | 9 +- docs/src/upgrade/release-3.7.x.asciidoc | 12 +- .../structure/io/graphson/GraphSONMapper.java | 175 ++++++++++++++++-- .../example/gadget/GraphSONTestGadgets.java | 34 ++++ ...GraphSONMapperPartialEmbeddedTypeTest.java | 69 +++++++ 5 files changed, 275 insertions(+), 24 deletions(-) diff --git a/docs/src/dev/io/graphson.asciidoc b/docs/src/dev/io/graphson.asciidoc index eb4bb37919b..00bf28b0690 100644 --- a/docs/src/dev/io/graphson.asciidoc +++ b/docs/src/dev/io/graphson.asciidoc @@ -114,9 +114,12 @@ link:https://github.com/FasterXML/jackson-databind[Jackson] type embedding appro name into a "@class" field in the JSON. While this approach isn't especially language agnostic it does at least give some hint as to what the expected type is. On read, that class name is constrained to a validated allow-list so that an untrusted document cannot name an arbitrary class for construction. A class outside the allowed set is refused. -The allow-list permits the `java.lang`, `java.util`, `java.math`, `java.time`, `java.sql` and `org.apache.tinkerpop` -packages plus array types. A provider or application reading trusted input can add its own package with -`GraphSONMapper.Builder.addAllowedTypeIdPrefix(String...)`, for example `addAllowedTypeIdPrefix("com.example.")`. +The allow-list permits classes in the `java.lang`, `java.util`, `java.math`, `java.time`, `java.sql` and +`org.apache.tinkerpop` packages (and their subpackages) plus array types, and by exact name a few safe value types +in otherwise-excluded packages (`java.net.InetAddress`, `java.net.URI`); the rest of `java.net` and `java.nio` is +excluded, and a parameterized type id or a `java.lang.Class` value is refused. A provider or application reading +trusted input can add its own package with `GraphSONMapper.Builder.addAllowedTypeIdPrefix(String...)`, for example +`addAllowedTypeIdPrefix("com.example.")`. This section focuses on non-embedded types and their formats as there was little usage of embedded types in generalized object serialization use cases. The format was simply too cumbersome to parse of non-Jackson enabled libraries and the diff --git a/docs/src/upgrade/release-3.7.x.asciidoc b/docs/src/upgrade/release-3.7.x.asciidoc index e721fe5fcb5..8339c015116 100644 --- a/docs/src/upgrade/release-3.7.x.asciidoc +++ b/docs/src/upgrade/release-3.7.x.asciidoc @@ -81,15 +81,19 @@ See: link:https://issues.apache.org/jira/browse/TINKERPOP-3278[TINKERPOP-3278] GraphSON 1.0 with embedded types (`TypeInfo.PARTIAL_TYPES`) reconstructs a value from the Java class named in a `@class` property using Jackson default typing. That mechanism previously resolved any class named in the document, which is an unsafe-deserialization vector when reading untrusted input. Default typing is now constrained by a -`PolymorphicTypeValidator` that permits only the value packages GraphSON 1.0 legitimately round-trips (`java.lang`, -`java.util`, `java.math`, `java.time`, `java.sql`, `org.apache.tinkerpop`) plus array types. Standard scalar, collection and -array values are unaffected. +`PolymorphicTypeValidator` that allows classes in the value packages GraphSON 1.0 legitimately round-trips +(`java.lang`, `java.util`, `java.math`, `java.time`, `java.sql`, `org.apache.tinkerpop`, and their subpackages) plus +array types, and by exact name a few safe value types in otherwise-excluded packages (`java.net.InetAddress`, +`java.net.URI`). The rest of `java.net` and `java.nio` is excluded, and a parameterized type id or a +`java.lang.Class` value is refused. Standard scalar, collection and array values are unaffected. Some value types +are written by V1 as a concrete subtype Jackson cannot reconstruct (`java.nio.ByteBuffer`); this was never +round-trippable in V1 and remains refused on read. A document whose `@class` names a type outside that set is now refused rather than instantiated: [source,text] ---- -Could not resolve subtype ...: Configured `PolymorphicTypeValidator` ... denied resolution +Could not resolve type id ...: Configured `PolymorphicTypeValidator` ... denied resolution ---- Applications that deliberately read a custom or provider value type through GraphSON 1.0 embedded types (for diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapper.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapper.java index 053044ab72e..54db3755406 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapper.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapper.java @@ -28,8 +28,12 @@ import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper; import org.apache.tinkerpop.shaded.jackson.databind.SerializationFeature; import org.apache.tinkerpop.shaded.jackson.databind.JavaType; +import org.apache.tinkerpop.shaded.jackson.databind.DatabindContext; +import org.apache.tinkerpop.shaded.jackson.databind.DeserializationContext; import org.apache.tinkerpop.shaded.jackson.databind.cfg.MapperConfig; import org.apache.tinkerpop.shaded.jackson.databind.jsontype.PolymorphicTypeValidator; +import org.apache.tinkerpop.shaded.jackson.databind.jsontype.TypeIdResolver; +import org.apache.tinkerpop.shaded.jackson.databind.jsontype.NamedType; import org.apache.tinkerpop.shaded.jackson.databind.jsontype.TypeResolverBuilder; import org.apache.tinkerpop.shaded.jackson.databind.jsontype.impl.StdTypeResolverBuilder; import org.apache.tinkerpop.shaded.jackson.databind.module.SimpleModule; @@ -38,12 +42,18 @@ import java.sql.Timestamp; import java.util.ArrayList; +import java.util.Collection; +import java.io.IOException; import java.util.Arrays; import java.util.Calendar; import java.util.Date; import java.util.List; import java.util.Map; import java.util.TimeZone; +import java.util.LinkedHashSet; +import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; import java.util.UUID; /** @@ -67,11 +77,21 @@ */ public class GraphSONMapper implements Mapper { - // GraphSON 1.0 embedded types reconstruct a value from the class named in a "@class" property via Jackson - // default typing; constrain it to safe packages so an untrusted payload cannot name an arbitrary class. - // Providers can extend this with Builder.addAllowedTypeIdPrefix(...) for trusted input. - private static final List GRAPHSON_1_0_DEFAULT_TYPE_PREFIXES = Arrays.asList( - "java.lang.", "java.util.", "java.math.", "java.time.", "java.sql.", "org.apache.tinkerpop."); + // Java base value types registered for GraphSON 2.0/3.0; shared by registerJavaBaseTypes and the V1 allow-list. + private static final List GRAPHSON_JAVA_BASE_TYPES = Arrays.asList( + UUID.class, Class.class, Calendar.class, Date.class, TimeZone.class, Timestamp.class); + + // Network packages removed from the derived allow-list: their classes can act on their value during + // deserialization (e.g. java.net.URL performs DNS lookups). + private static final List GRAPHSON_1_0_DENIED_TYPE_PREFIXES = Arrays.asList("java.net.", "java.nio."); + // exact class names refused even though their package prefix is allowed; a java.lang.Class *value* would + // otherwise load and initialize any class named in it (Jackson resolves it with initialize=true). + private static final List GRAPHSON_1_0_DENIED_EXACT_TYPES = Arrays.asList("java.lang.Class"); + private static final List GRAPHSON_1_0_DEFAULT_TYPE_PREFIXES = graphSON1dDefaultTypePrefixes(); + // safe value types V1 round-trips that GraphSON 2.0/3.0 do not register (so the derivation misses them); + // java.net.URI is string-backed and performs no DNS lookup, unlike java.net.URL. + private static final List GRAPHSON_1_0_ALLOWED_EXACT_EXTRA = Arrays.asList("java.net.URI"); + private static final Set GRAPHSON_1_0_ALLOWED_EXACT_TYPES = graphSON1dAllowedExactTypes(); public static final int DEFAULT_MAX_NUMBER_LENGTH = 10000; private final List customModules; @@ -147,6 +167,15 @@ public ObjectMapper createMapper() { public PolymorphicTypeValidator subTypeValidator(final MapperConfig config) { return typeValidator; } + + @Override + protected TypeIdResolver idResolver(final MapperConfig config, final JavaType baseType, + final PolymorphicTypeValidator subtypeValidator, + final Collection subtypes, + final boolean forSer, final boolean forDeser) { + return new GraphSON1dScreeningIdResolver( + super.idResolver(config, baseType, subtypeValidator, subtypes, forSer, forDeser)); + } }.init(JsonTypeInfo.Id.CLASS, null) .inclusion(JsonTypeInfo.As.PROPERTY) .typeProperty(GraphSONTokens.CLASS); @@ -171,10 +200,11 @@ public PolymorphicTypeValidator subTypeValidator(final MapperConfig config) { } /** - * A {@link PolymorphicTypeValidator} for GraphSON 1.0 embedded types that decides purely from the type id name, - * so a disallowed class is refused before it is loaded (a name that only fails the check post-load would still - * have its static initializer run). A name is allowed when it, or an array's element type, starts with one of - * the trusted prefixes; primitive arrays are allowed. + * A {@link PolymorphicTypeValidator} for GraphSON 1.0 embedded types that decides a simple type id purely from + * its name, so a disallowed class is refused before it is loaded. A name is allowed when it, or an array's + * element type, starts with one of the trusted prefixes and is not an exact-denied class; primitive arrays are + * allowed. Parameterized type ids are handled separately by {@link GraphSON1dScreeningIdResolver}, since the + * validator is not shown a type id's arguments. */ private static PolymorphicTypeValidator graphSON1dTypeValidator(final List allowedPrefixes) { return new PolymorphicTypeValidator.Base() { @@ -198,6 +228,119 @@ public Validity validateSubType(final MapperConfig config, final JavaType bas }; } + /** + * Wraps the class-name {@link TypeIdResolver} so a parameterized GraphSON 1.0 type id (one containing + * '{@code <}') is refused before Jackson resolves it. Jackson validates a simple id's name up front, but for a + * parameterized id it loads every type argument before validating and skips validation of enum arguments + * entirely; GraphSON 1.0 never emits a parameterized id, so any id containing '{@code <}' is rejected here. + */ + private static final class GraphSON1dScreeningIdResolver implements TypeIdResolver { + private final TypeIdResolver delegate; + private JavaType baseType; + + private GraphSON1dScreeningIdResolver(final TypeIdResolver delegate) { + this.delegate = delegate; + } + + @Override + public void init(final JavaType baseType) { + this.baseType = baseType; + delegate.init(baseType); + } + + @Override + public JavaType typeFromId(final DatabindContext context, final String id) throws IOException { + if (id.indexOf('<') >= 0) { + if (context instanceof DeserializationContext) + throw ((DeserializationContext) context).invalidTypeIdException(baseType, id, + "GraphSON 1.0 does not permit a parameterized type id"); + throw new IOException("GraphSON 1.0 does not permit a parameterized type id: " + id); + } + return delegate.typeFromId(context, id); + } + + @Override + public String idFromValue(final Object value) { + return delegate.idFromValue(value); + } + + @Override + public String idFromValueAndType(final Object value, final Class suggestedType) { + return delegate.idFromValueAndType(value, suggestedType); + } + + @Override + public String idFromBaseType() { + return delegate.idFromBaseType(); + } + + @Override + public String getDescForKnownTypeIds() { + return delegate.getDescForKnownTypeIds(); + } + + @Override + public JsonTypeInfo.Id getMechanism() { + return delegate.getMechanism(); + } + } + + // the types GraphSON 2.0/3.0 register: shared java base types plus the core and extended 2.0 and 3.0 type + // definitions. TypeInfo does not affect the (static) type-definition map, so NO_TYPES is passed. + private static Set registeredV2V3Types() { + final Set registered = new LinkedHashSet<>(GRAPHSON_JAVA_BASE_TYPES); + registered.addAll(GraphSONVersion.V2_0.getBuilder().create(false, TypeInfo.NO_TYPES) + .getTypeDefinitions().keySet()); + registered.addAll(GraphSONVersion.V3_0.getBuilder().create(false, TypeInfo.NO_TYPES) + .getTypeDefinitions().keySet()); + registered.addAll(GraphSONXModuleV2.build().create(false, TypeInfo.NO_TYPES) + .getTypeDefinitions().keySet()); + registered.addAll(GraphSONXModuleV3.build().create(false, TypeInfo.NO_TYPES) + .getTypeDefinitions().keySet()); + return registered; + } + + private static String graphSON1dPackagePrefix(final Class c) { + final String pkg = c.getPackage().getName(); + return pkg.startsWith("org.apache.tinkerpop") ? "org.apache.tinkerpop." : pkg + "."; + } + + /** + * Derives the GraphSON 1.0 embedded-type allow-list from the packages of the types GraphSON 2.0/3.0 register, + * with the denied network packages removed. TinkerPop's own types collapse to a single + * {@code org.apache.tinkerpop.} prefix because V1 names concrete subclasses in various subpackages. + */ + private static List graphSON1dDefaultTypePrefixes() { + final SortedSet prefixes = new TreeSet<>(); + for (final Class c : registeredV2V3Types()) { + if (c.getPackage() == null) + continue; + final String prefix = graphSON1dPackagePrefix(c); + if (GRAPHSON_1_0_DENIED_TYPE_PREFIXES.stream().noneMatch(prefix::startsWith)) + prefixes.add(prefix); + } + return new ArrayList<>(prefixes); + } + + /** + * Value types allowed by exact name even though their package is denied, so a usable value type is not lost to + * the coarser package denial while dangerous siblings (java.net.URL and the rest) stay refused. This is the + * classes GraphSON 2.0/3.0 register in a denied package (for example java.net.InetAddress) plus a few safe + * string-backed types they do not register ({@link #GRAPHSON_1_0_ALLOWED_EXACT_EXTRA}, e.g. java.net.URI). + * Some entries (java.nio.ByteBuffer) are written by V1 as a concrete subtype Jackson cannot reconstruct, so + * they remain effectively unsupported in V1 regardless. + */ + private static Set graphSON1dAllowedExactTypes() { + final SortedSet exact = new TreeSet<>(GRAPHSON_1_0_ALLOWED_EXACT_EXTRA); + for (final Class c : registeredV2V3Types()) { + if (c.getPackage() == null) + continue; + if (GRAPHSON_1_0_DENIED_TYPE_PREFIXES.stream().anyMatch(graphSON1dPackagePrefix(c)::startsWith)) + exact.add(c.getName()); + } + return exact; + } + private static boolean isAllowedTypeName(final String typeName, final List allowedPrefixes) { // unwrap array descriptors: "[Ljava.io.File;" -> "java.io.File", "[[B" -> primitive element String name = typeName; @@ -207,6 +350,10 @@ private static boolean isAllowedTypeName(final String typeName, final List graphSONTypeIdResolver.addCustomType(String.format("%s:%s", GraphSONTokens.GREMLIN_TYPE_NAMESPACE, e.getSimpleName()), e)); + GRAPHSON_JAVA_BASE_TYPES.forEach(e -> graphSONTypeIdResolver.addCustomType( + String.format("%s:%s", GraphSONTokens.GREMLIN_TYPE_NAMESPACE, e.getSimpleName()), e)); } public static class Builder implements Mapper.Builder { diff --git a/gremlin-core/src/test/java/com/example/gadget/GraphSONTestGadgets.java b/gremlin-core/src/test/java/com/example/gadget/GraphSONTestGadgets.java index 90682aebba5..17940f1f8b8 100644 --- a/gremlin-core/src/test/java/com/example/gadget/GraphSONTestGadgets.java +++ b/gremlin-core/src/test/java/com/example/gadget/GraphSONTestGadgets.java @@ -64,4 +64,38 @@ public int hashCode() { return Objects.hashCode(x); } } + + /** + * Enum counterpart of {@link StaticInitCanary}, used to verify an enum named as a generic type argument is + * refused before it is loaded (Jackson otherwise skips validation of enum type arguments). + */ + public enum StaticInitCanaryEnum { + A, B; + public static final String FIRED_PROPERTY = "tinkerpop.test.graphson.staticInitCanaryEnum"; + static { + System.setProperty(FIRED_PROPERTY, "fired"); + } + } + + /** + * Canary used to verify a disallowed class named as a generic type argument is not loaded when refused. + */ + public static class StaticInitCanaryArg { + public static final String FIRED_PROPERTY = "tinkerpop.test.graphson.staticInitCanaryArg"; + static { + System.setProperty(FIRED_PROPERTY, "fired"); + } + public int x; + } + + /** + * Canary used to verify a java.lang.Class value cannot name and load an arbitrary class. + */ + public static class StaticInitCanaryValue { + public static final String FIRED_PROPERTY = "tinkerpop.test.graphson.staticInitCanaryValue"; + static { + System.setProperty(FIRED_PROPERTY, "fired"); + } + public int x; + } } diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperPartialEmbeddedTypeTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperPartialEmbeddedTypeTest.java index 4850921bea1..1a2c308a5e3 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperPartialEmbeddedTypeTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperPartialEmbeddedTypeTest.java @@ -26,6 +26,9 @@ import org.apache.tinkerpop.shaded.jackson.databind.JsonMappingException; import com.example.gadget.GraphSONTestGadgets.SamplePojo; import com.example.gadget.GraphSONTestGadgets.StaticInitCanary; +import com.example.gadget.GraphSONTestGadgets.StaticInitCanaryArg; +import com.example.gadget.GraphSONTestGadgets.StaticInitCanaryEnum; +import com.example.gadget.GraphSONTestGadgets.StaticInitCanaryValue; import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper; import org.apache.tinkerpop.shaded.jackson.databind.exc.InvalidTypeIdException; import org.junit.Test; @@ -260,6 +263,15 @@ public void shouldRejectDisallowedClassWithEmbedTypeSettingV1() { "{\"@class\":\"java.util.HashMap\",\"evil\":{\"@class\":\"java.io.File\",\"path\":\"/tmp/x\"}}"); } + @Test + public void shouldRejectNetworkPackageTypeWithEmbedTypeSettingV1() { + // java.net (and java.nio) are intentionally excluded even though GraphSON 2.0/3.0 register + // java.net.InetAddress / java.nio.ByteBuffer, because a class such as java.net.URL performs DNS lookups + // during deserialization; this pins that exclusion against the registry-derived allow-list + assertDeniedByTypeValidator(v1Typed(), + "{\"@class\":\"java.util.HashMap\",\"evil\":{\"@class\":\"java.net.URL\",\"u\":\"http://example.com\"}}"); + } + @Test public void shouldRoundTripArraysWithEmbedTypeSettingV1() throws Exception { // arrays are legitimate GraphSON 1.0 values and rely on default typing; they must still round-trip @@ -318,6 +330,39 @@ public void shouldAllowConfiguredTypeIdPrefixWithEmbedTypeSettingV1() throws Exc assertEquals(new SamplePojo(42), read.get("p")); } + @Test + public void shouldRejectEnumTypeParameterAndNotLoadItV1() { + // an enum named as a generic type argument must be refused before it is loaded; Jackson otherwise skips + // validation of enum type arguments entirely, which would be a full allow-list bypass + System.clearProperty(StaticInitCanaryEnum.FIRED_PROPERTY); + assertDeniedByTypeValidator(v1Typed(), + "{\"@class\":\"java.util.HashMap\",\"A\":\"v\"}"); + assertNull("an enum type argument must not be class-loaded when refused", + System.getProperty(StaticInitCanaryEnum.FIRED_PROPERTY)); + } + + @Test + public void shouldNotLoadDisallowedTypeParameterWhenRefusingV1() { + // a disallowed class named as a generic type argument must be refused before it is loaded (deny-before-load); + // a parameterized type id is refused up front rather than after its arguments are resolved + System.clearProperty(StaticInitCanaryArg.FIRED_PROPERTY); + assertDeniedByTypeValidator(v1Typed(), + "{\"@class\":\"java.util.HashMap\",\"k\":\"v\"}"); + assertNull("a disallowed type argument must not be class-loaded when refused", + System.getProperty(StaticInitCanaryArg.FIRED_PROPERTY)); + } + + @Test + public void shouldRejectClassValueAndNotLoadItV1() { + // a java.lang.Class *value* must not be able to name and load an arbitrary class (java.lang.Class is + // exact-denied even though the java.lang package is otherwise allowed) + System.clearProperty(StaticInitCanaryValue.FIRED_PROPERTY); + assertDeniedByTypeValidator(v1Typed(), + "{\"@class\":\"java.util.HashMap\",\"c\":[\"java.lang.Class\",\"com.example.gadget.GraphSONTestGadgets$StaticInitCanaryValue\"]}"); + assertNull("a java.lang.Class value must not load the class it names", + System.getProperty(StaticInitCanaryValue.FIRED_PROPERTY)); + } + @Test public void shouldNotLoadDisallowedClassWhenRefusingV1() { // a refused @class must be denied by name before it is loaded, otherwise its static initializer runs @@ -328,6 +373,30 @@ public void shouldNotLoadDisallowedClassWhenRefusingV1() { System.getProperty(StaticInitCanary.FIRED_PROPERTY)); } + @Test + public void shouldRoundTripInetAddressWithEmbedTypeSettingV1() throws Exception { + // java.net.InetAddress is a value type GraphSON 2.0/3.0 support; it is allowed by exact name even though + // the java.net package is otherwise denied, so it must still round-trip without an opt-out + final ObjectMapper mapper = v1Typed(); + final Map m = new HashMap<>(); + m.put("a", java.net.InetAddress.getByAddress(new byte[]{127, 0, 0, 1})); + + final Map read = mapper.readValue(mapper.writeValueAsString(m), HashMap.class); + assertEquals(java.net.InetAddress.getByAddress(new byte[]{127, 0, 0, 1}), read.get("a")); + } + + @Test + public void shouldRoundTripUriWithEmbedTypeSettingV1() throws Exception { + // java.net.URI is string-backed and performs no DNS lookup; it is allowed by exact name even though the + // java.net package is otherwise denied, so it must still round-trip without an opt-out + final ObjectMapper mapper = v1Typed(); + final Map m = new HashMap<>(); + m.put("u", new java.net.URI("http://example.com/x")); + + final Map read = mapper.readValue(mapper.writeValueAsString(m), HashMap.class); + assertEquals(new java.net.URI("http://example.com/x"), read.get("u")); + } + private static ObjectMapper v1Typed() { return GraphSONMapper.build().version(GraphSONVersion.V1_0).typeInfo(TypeInfo.PARTIAL_TYPES).create().createMapper(); }