From 41f42a92c269f8c369a11d16913656956808a239 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Tue, 1 Sep 2026 19:03:10 +0100 Subject: [PATCH] fix: don't dereference a null superclass in the protobuf allow list check Motivation: ProtobufSerializer.isInAllowListClassName reads clazz.getSuperclass.getName unconditionally. getSuperclass is null for an interface, for Object and for a primitive, and the class it checks is the manifest from the wire, so a manifest naming an interface raised NullPointerException from the allow list check rather than the IllegalArgumentException that refusing a class is meant to produce. isInAllowList also evaluated isBoundToProtobufSerializer first, which calls serializerFor and raises, filling in a stack trace, for a class that is not bound - the common case for a class allowed only by name. Modification: Skip the superclass when there is none, and test the name list before the binding, which cannot throw. Both operands are pure predicates so the decision is unchanged. Result: An interface or Object manifest is refused with the allow list error instead of a NullPointerException. --- .../serialization/ProtobufSerializer.scala | 9 +++++++-- .../serialization/ProtobufSerializerSpec.scala | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/remote/src/main/scala/org/apache/pekko/remote/serialization/ProtobufSerializer.scala b/remote/src/main/scala/org/apache/pekko/remote/serialization/ProtobufSerializer.scala index e2503d11bc9..c45442cbec4 100644 --- a/remote/src/main/scala/org/apache/pekko/remote/serialization/ProtobufSerializer.scala +++ b/remote/src/main/scala/org/apache/pekko/remote/serialization/ProtobufSerializer.scala @@ -181,7 +181,10 @@ class ProtobufSerializer(val system: ExtendedActorSystem) extends BaseSerializer * and still bind with the same class (interface). */ private def isInAllowList(clazz: Class[?]): Boolean = { - isBoundToProtobufSerializer(clazz) || isInAllowListClassName(clazz) + // The name check comes first because it cannot throw: `isBoundToProtobufSerializer` calls + // `serializerFor`, which raises (and fills in the stack trace of) a NotSerializableException + // for a class that is not bound. + isInAllowListClassName(clazz) || isBoundToProtobufSerializer(clazz) } private def isBoundToProtobufSerializer(clazz: Class[?]): Boolean = { @@ -194,8 +197,10 @@ class ProtobufSerializer(val system: ExtendedActorSystem) extends BaseSerializer } private def isInAllowListClassName(clazz: Class[?]): Boolean = { + // getSuperclass is null for an interface, for Object and for a primitive, and the manifest + // class comes off the wire, so it can be any of those allowedClassNames(clazz.getName) || - allowedClassNames(clazz.getSuperclass.getName) || + ((clazz.getSuperclass ne null) && allowedClassNames(clazz.getSuperclass.getName)) || clazz.getInterfaces.exists(c => allowedClassNames(c.getName)) } } diff --git a/remote/src/test/scala/org/apache/pekko/remote/serialization/ProtobufSerializerSpec.scala b/remote/src/test/scala/org/apache/pekko/remote/serialization/ProtobufSerializerSpec.scala index 3b9885398e2..9d79c066bca 100644 --- a/remote/src/test/scala/org/apache/pekko/remote/serialization/ProtobufSerializerSpec.scala +++ b/remote/src/test/scala/org/apache/pekko/remote/serialization/ProtobufSerializerSpec.scala @@ -123,6 +123,24 @@ class ProtobufSerializerSpec extends PekkoSpec(s""" } } + "reject an interface manifest rather than failing on its missing superclass" in { + // getSuperclass is null for an interface, and the manifest class comes off the wire. + // This used to raise NullPointerException from the allow list check instead of the + // IllegalArgumentException that refusing the class is supposed to produce. + val originalSerializer = ser.serializerFor(classOf[MyMessage]) + intercept[IllegalArgumentException] { + ser.deserialize(Array[Byte](), originalSerializer.identifier, classOf[Runnable].getName).get + }.getMessage should include("allow list") + } + + "reject java.lang.Object as a manifest" in { + // Object.getSuperclass is null too + val originalSerializer = ser.serializerFor(classOf[MyMessage]) + intercept[IllegalArgumentException] { + ser.deserialize(Array[Byte](), originalSerializer.identifier, classOf[Object].getName).get + }.getMessage should include("allow list") + } + "allow deserialization of classes in configured allowed classes" in { val originalSerializer = ser.serializerFor(classOf[MyMessage])