This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko.git
The following commit(s) were added to refs/heads/main by this push:
new 67c3b557e5 fix: resolve, check and parse manifests more carefully in
the Jackson serializers (#3503)
67c3b557e5 is described below
commit 67c3b557e538416fe55b9cbe3745817dbb4b6413
Author: PJ Fanning <[email protected]>
AuthorDate: Thu Sep 3 09:16:28 2026 +0100
fix: resolve, check and parse manifests more carefully in the Jackson
serializers (#3503)
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 | 30 +++++++++++---
.../jackson/CaseObjectInitializationProbe.scala | 35 ++++++++++++++++
.../jackson/JacksonSerializerSpec.scala | 48 ++++++++++++++++++++++
.../serialization/jackson3/JacksonSerializer.scala | 30 +++++++++++---
.../jackson3/CaseObjectInitializationProbe.scala | 35 ++++++++++++++++
.../jackson3/JacksonSerializerSpec.scala | 48 ++++++++++++++++++++++
6 files changed, 216 insertions(+), 10 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 82fb9cd3f8..01fa6d016f 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
@@ -347,14 +348,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
@@ -460,7 +470,10 @@ import pekko.util.OptionVal
* and still bind with the same class (interface).
*/
private def isInAllowList(clazz: Class[?]): Boolean = {
- isBoundToJacksonSerializer(clazz) || hasAllowedClassPrefix(clazz.getName)
+ // 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 = {
@@ -509,7 +522,14 @@ 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
+ manifest
+ .substring(i + 1)
+ .toIntOption
+ .getOrElse(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 b2b6c2eb2c..1265562c95 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
@@ -856,6 +857,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
@@ -1311,6 +1315,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/main/scala/org/apache/pekko/serialization/jackson3/JacksonSerializer.scala
b/serialization-jackson3/src/main/scala/org/apache/pekko/serialization/jackson3/JacksonSerializer.scala
index 6a9db1da2c..9c6181d358 100644
---
a/serialization-jackson3/src/main/scala/org/apache/pekko/serialization/jackson3/JacksonSerializer.scala
+++
b/serialization-jackson3/src/main/scala/org/apache/pekko/serialization/jackson3/JacksonSerializer.scala
@@ -120,7 +120,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
@@ -348,14 +349,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
@@ -461,7 +471,10 @@ import pekko.util.OptionVal
* and still bind with the same class (interface).
*/
private def isInAllowList(clazz: Class[?]): Boolean = {
- isBoundToJacksonSerializer(clazz) || hasAllowedClassPrefix(clazz.getName)
+ // 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 = {
@@ -510,7 +523,14 @@ 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
+ manifest
+ .substring(i + 1)
+ .toIntOption
+ .getOrElse(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-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
+}
diff --git
a/serialization-jackson3/src/test/scala/org/apache/pekko/serialization/jackson3/JacksonSerializerSpec.scala
b/serialization-jackson3/src/test/scala/org/apache/pekko/serialization/jackson3/JacksonSerializerSpec.scala
index 8338f13c40..551e22426c 100644
---
a/serialization-jackson3/src/test/scala/org/apache/pekko/serialization/jackson3/JacksonSerializerSpec.scala
+++
b/serialization-jackson3/src/test/scala/org/apache/pekko/serialization/jackson3/JacksonSerializerSpec.scala
@@ -14,6 +14,7 @@
package org.apache.pekko.serialization.jackson3
import java.lang
+import java.io.NotSerializableException
import java.nio.charset.StandardCharsets
import java.time.{ Duration, Instant, LocalDateTime }
import java.time.temporal.ChronoUnit
@@ -801,6 +802,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.jackson3.NotAllowedCaseObject$"
+
def serializerFor(obj: AnyRef, sys: ActorSystem = system): JacksonSerializer
=
serialization(sys).findSerializerFor(obj) match {
case serializer: JacksonSerializer => serializer
@@ -1256,6 +1260,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
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
