This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/pekko.git
The following commit(s) were added to refs/heads/1.7.x by this push:
new 422ad44d13 fix: resolve, check and parse manifests more carefully in
the Jackson serializers (#3503) (#3517)
422ad44d13 is described below
commit 422ad44d134af59afe9ac53dd69896bbf5f43a5e
Author: PJ Fanning <[email protected]>
AuthorDate: Fri Sep 4 11:26:25 2026 +0100
fix: resolve, check and parse manifests more carefully in the Jackson
serializers (#3503) (#3517)
Motivation:
Four issues in JacksonSerializer, all reachable from a wire manifest:
- The case object branch called getObjectFor before checkAllowedClass.
getObjectFor reads the MODULE$ field, which initializes the class, so a
manifest naming a class the allow list would reject ran that class's
initializer on the way to being rejected.
- LZ4Meta.get checked for 4 remaining bytes and then read 8, so a 4 to 7
byte payload beginning with the LZ4 magic raised BufferUnderflowException.
- parseManifest called toInt on whatever followed the last '#', so a
non-numeric version raised NumberFormatException.
- isInAllowList evaluated isBoundToJacksonSerializer first, which calls
serializerFor and raises (filling in a stack trace) for a class that is
allowed only by prefix. checkAllowedClass runs on every fromBinary.
Modification:
Resolve the case object's class with getClassFor, which does not initialize,
run checkAllowedClass on it, and only then read the module field. Require 8
remaining bytes for an LZ4 header. Parse the manifest version with
toIntOption and report a bad one as NotSerializableException. Test the
prefix before the binding, which cannot throw. Applied to both
serialization-jackson and serialization-jackson3.
Result:
A rejected manifest no longer initializes the class it names, two malformed
manifests are reported as serialization failures rather than as unrelated
runtime exceptions, and the allow list check no longer builds an exception
per message on the prefix path. No change for manifests that were accepted
before.
---
.../serialization/jackson/JacksonSerializer.scala | 34 ++++++++++++---
.../jackson/CaseObjectInitializationProbe.scala | 35 ++++++++++++++++
.../jackson/JacksonSerializerSpec.scala | 48 ++++++++++++++++++++++
.../jackson3/CaseObjectInitializationProbe.scala | 35 ++++++++++++++++
4 files changed, 146 insertions(+), 6 deletions(-)
diff --git
a/serialization-jackson/src/main/scala/org/apache/pekko/serialization/jackson/JacksonSerializer.scala
b/serialization-jackson/src/main/scala/org/apache/pekko/serialization/jackson/JacksonSerializer.scala
index ebadef6a7c..2799c807eb 100644
---
a/serialization-jackson/src/main/scala/org/apache/pekko/serialization/jackson/JacksonSerializer.scala
+++
b/serialization-jackson/src/main/scala/org/apache/pekko/serialization/jackson/JacksonSerializer.scala
@@ -119,7 +119,8 @@ import pekko.util.OptionVal
}
def get(buffer: ByteBuffer): OptionVal[LZ4Meta] = {
- if (buffer.remaining() < 4) {
+ // the header is the magic plus the declared length, so 8 bytes are read
below
+ if (buffer.remaining() < 8) {
OptionVal.None
} else if (buffer.getInt() != LZ4_MAGIC) {
OptionVal.None
@@ -346,14 +347,23 @@ import pekko.util.OptionVal
checkAllowedClassName(className)
if (isCaseObject(className)) {
+ // `getObjectFor` reads the MODULE$ field, which initializes the class.
Resolve the class
+ // with `getClassFor` first, which does not initialize, and run the
allow list check on it,
+ // so a manifest naming a class this serializer would reject cannot run
that class's
+ // initializer on the way to being rejected.
+ val clazz = system.dynamicAccess.getClassFor[AnyRef](className) match {
+ case Success(c) => c
+ case Failure(_) =>
+ throw new NotSerializableException(
+ s"Cannot find manifest case object [$className] for serializer
[${getClass.getName}].")
+ }
+ checkAllowedClass(clazz)
val result = system.dynamicAccess.getObjectFor[AnyRef](className) match {
case Success(obj) => obj
case Failure(_) =>
throw new NotSerializableException(
s"Cannot find manifest case object [$className] for serializer
[${getClass.getName}].")
}
- val clazz = result.getClass
- checkAllowedClass(clazz)
// no migrations for case objects, since no json tree
logFromBinaryDuration(bytes, bytes, startTime, clazz)
result
@@ -458,8 +468,11 @@ import pekko.util.OptionVal
* That is also possible when changing a binding from a JacksonSerializer to
another serializer (e.g. protobuf)
* and still bind with the same class (interface).
*/
- private def isInAllowList(clazz: Class[_]): Boolean = {
- isBoundToJacksonSerializer(clazz) || hasAllowedClassPrefix(clazz.getName)
+ private def isInAllowList(clazz: Class[?]): Boolean = {
+ // The prefix check comes first because it cannot throw:
`isBoundToJacksonSerializer` calls
+ // `serializerFor`, which raises (and fills in the stack trace of) a
NotSerializableException
+ // for an unbound class, and this runs on every `fromBinary`.
+ hasAllowedClassPrefix(clazz.getName) || isBoundToJacksonSerializer(clazz)
}
private def isBoundToJacksonSerializer(clazz: Class[_]): Boolean = {
@@ -508,7 +521,16 @@ import pekko.util.OptionVal
private def parseManifest(manifest: String) = {
val i = manifest.lastIndexOf('#')
- val fromVersion = if (i == -1) 1 else manifest.substring(i + 1).toInt
+ val fromVersion =
+ if (i == -1) 1
+ else
+ // not toIntOption, which Scala 2.12 does not have
+ try manifest.substring(i + 1).toInt
+ catch {
+ case _: NumberFormatException =>
+ throw new NotSerializableException(
+ s"Manifest [$manifest] for serializer [${getClass.getName}] does
not end with a numeric version.")
+ }
val manifestClassName = if (i == -1) manifest else manifest.substring(0, i)
(fromVersion, manifestClassName)
}
diff --git
a/serialization-jackson/src/test/scala/org/apache/pekko/serialization/jackson/CaseObjectInitializationProbe.scala
b/serialization-jackson/src/test/scala/org/apache/pekko/serialization/jackson/CaseObjectInitializationProbe.scala
new file mode 100644
index 0000000000..3da2ef5ec4
--- /dev/null
+++
b/serialization-jackson/src/test/scala/org/apache/pekko/serialization/jackson/CaseObjectInitializationProbe.scala
@@ -0,0 +1,35 @@
+/*
+ * 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 org.apache.pekko.serialization.jackson
+
+/**
+ * Records whether [[NotAllowedCaseObject]] has been initialized. A test
asserts this stays
+ * false while deserializing a manifest that names it, so refer to that object
by its class
+ * name as a string and never in code, which would initialize it.
+ */
+object CaseObjectInitializationProbe {
+ @volatile var initialized: Boolean = false
+}
+
+/**
+ * Neither bound to a Jackson serializer nor covered by
`allowed-class-prefix`, so a manifest
+ * naming it has to be rejected by the allow list.
+ */
+object NotAllowedCaseObject {
+ CaseObjectInitializationProbe.initialized = true
+}
diff --git
a/serialization-jackson/src/test/scala/org/apache/pekko/serialization/jackson/JacksonSerializerSpec.scala
b/serialization-jackson/src/test/scala/org/apache/pekko/serialization/jackson/JacksonSerializerSpec.scala
index 60fab76c93..c664dd1eab 100644
---
a/serialization-jackson/src/test/scala/org/apache/pekko/serialization/jackson/JacksonSerializerSpec.scala
+++
b/serialization-jackson/src/test/scala/org/apache/pekko/serialization/jackson/JacksonSerializerSpec.scala
@@ -14,6 +14,7 @@
package org.apache.pekko.serialization.jackson
import java.lang
+import java.io.NotSerializableException
import java.nio.charset.StandardCharsets
import java.time.Duration
import java.time.Instant
@@ -818,6 +819,9 @@ abstract class JacksonSerializerSpec(serializerName: String)
serialization(sys).deserialize(blob, serializerId, manifest).get
}
+ // referenced by name only: mentioning the object in code would initialize it
+ val NotAllowedCaseObjectName =
"org.apache.pekko.serialization.jackson.NotAllowedCaseObject$"
+
def serializerFor(obj: AnyRef, sys: ActorSystem = system): JacksonSerializer
=
serialization(sys).findSerializerFor(obj) match {
case serializer: JacksonSerializer => serializer
@@ -1273,6 +1277,50 @@ abstract class JacksonSerializerSpec(serializerName:
String)
}
}
+ "not allow deserialization of a case object that is not in
serialization-bindings" in {
+ withTransportInformation() { () =>
+ val msg = SimpleCommand("ok")
+ val serializer = serializerFor(msg)
+ val blob = serializer.toBinary(msg)
+ intercept[IllegalArgumentException] {
+ // maliciously changing manifest to a case object
+ serializer.fromBinary(blob, NotAllowedCaseObjectName)
+ }.getMessage.toLowerCase should include("allowed-class-prefix")
+ }
+ }
+
+ "not initialize a case object class it goes on to reject" in {
+ withTransportInformation() { () =>
+ val msg = SimpleCommand("ok")
+ val serializer = serializerFor(msg)
+ val blob = serializer.toBinary(msg)
+ CaseObjectInitializationProbe.initialized should ===(false)
+ intercept[IllegalArgumentException] {
+ serializer.fromBinary(blob, NotAllowedCaseObjectName)
+ }
+ // reading MODULE$ would have run the object's body
+ CaseObjectInitializationProbe.initialized should ===(false)
+ }
+ }
+
+ "reject a manifest whose version is not a number" in {
+ withTransportInformation() { () =>
+ val msg = SimpleCommand("ok")
+ val serializer = serializerFor(msg)
+ val blob = serializer.toBinary(msg)
+ intercept[NotSerializableException] {
+ serializer.fromBinary(blob, classOf[SimpleCommand].getName +
"#not-a-number")
+ }.getMessage should include("numeric version")
+ }
+ }
+
+ "not underflow on a payload that is only as long as the LZ4 magic" in {
+ // the LZ4 header is the magic plus a 4 byte length, so 4 bytes cannot
be an LZ4 payload
+ JacksonSerializer.isLZ4(Array(0x87, 0xD9, 0x6D, 0xF6).map(_.toByte))
should ===(false)
+
JacksonSerializer.isLZ4(JacksonSerializer.LZ4Meta(Array.emptyByteArray).prependTo(Array.emptyByteArray))
should
+ ===(true)
+ }
+
"not allow serialization-bindings of open-ended types" in {
JacksonSerializer.disallowedSerializationBindings.foreach { clazz =>
val className = clazz.getName
diff --git
a/serialization-jackson3/src/test/scala/org/apache/pekko/serialization/jackson3/CaseObjectInitializationProbe.scala
b/serialization-jackson3/src/test/scala/org/apache/pekko/serialization/jackson3/CaseObjectInitializationProbe.scala
new file mode 100644
index 0000000000..3ef5ec25d4
--- /dev/null
+++
b/serialization-jackson3/src/test/scala/org/apache/pekko/serialization/jackson3/CaseObjectInitializationProbe.scala
@@ -0,0 +1,35 @@
+/*
+ * 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 org.apache.pekko.serialization.jackson3
+
+/**
+ * Records whether [[NotAllowedCaseObject]] has been initialized. A test
asserts this stays
+ * false while deserializing a manifest that names it, so refer to that object
by its class
+ * name as a string and never in code, which would initialize it.
+ */
+object CaseObjectInitializationProbe {
+ @volatile var initialized: Boolean = false
+}
+
+/**
+ * Neither bound to a Jackson serializer nor covered by
`allowed-class-prefix`, so a manifest
+ * naming it has to be rejected by the allow list.
+ */
+object NotAllowedCaseObject {
+ CaseObjectInitializationProbe.initialized = true
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
