From 9cec137a31fd3b3bac014c6b62d4196baad8605d Mon Sep 17 00:00:00 2001
From: Chris Rankin <chris.rankin@r3.com>
Date: Wed, 11 Oct 2017 11:17:14 +0100
Subject: [PATCH] CORDA-702: Don't whitelist certain non-annotated types
 (#1864)

* Don't whitelist arrays of non-serialisable types for RPC.
* Don't whitelist enums which have not been annotated as serialisable.
---
 .../serialization/CordaClassResolver.kt       |  5 ++-
 .../serialization/CordaClassResolverTests.kt  | 40 ++++++++++++++++++-
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolver.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolver.kt
index 04c43122ec..97c50dcd67 100644
--- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolver.kt
+++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolver.kt
@@ -57,12 +57,13 @@ class CordaClassResolver(serializationContext: SerializationContext) : DefaultCl
     private fun checkClass(type: Class<*>): Registration? {
         // If call path has disabled whitelisting (see [CordaKryo.register]), just return without checking.
         if (!whitelistEnabled) return null
-        // Allow primitives, abstracts and interfaces
-        if (type.isPrimitive || type == Any::class.java || isAbstract(type.modifiers) || type == String::class.java) return null
         // If array, recurse on element type
         if (type.isArray) return checkClass(type.componentType)
         // Specialised enum entry, so just resolve the parent Enum type since cannot annotate the specialised entry.
         if (!type.isEnum && Enum::class.java.isAssignableFrom(type)) return checkClass(type.superclass)
+        // Allow primitives, abstracts and interfaces. Note that we can also create abstract Enum types,
+        // but we don't want to whitelist those here.
+        if (type.isPrimitive || type == Any::class.java || type == String::class.java || (!type.isEnum && isAbstract(type.modifiers))) return null
         // It's safe to have the Class already, since Kryo loads it with initialisation off.
         // If we use a whitelist with blacklisting capabilities, whitelist.hasListed(type) may throw an IllegalStateException if input class is blacklisted.
         // Thus, blacklisting precedes annotation checking.
diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolverTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolverTests.kt
index 74a3d574ae..e684ef1f29 100644
--- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolverTests.kt
+++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/CordaClassResolverTests.kt
@@ -34,6 +34,23 @@ enum class Foo {
     abstract val value: Int
 }
 
+enum class BadFood {
+    Mud {
+        override val value = -1
+    };
+
+    abstract val value: Int
+}
+
+@CordaSerializable
+enum class Simple {
+    Easy
+}
+
+enum class BadSimple {
+    Nasty
+}
+
 @CordaSerializable
 open class Element
 
@@ -106,17 +123,36 @@ class CordaClassResolverTests {
 
     @Test
     fun `Annotation on enum works for specialised entries`() {
-        // TODO: Remove this suppress when we upgrade to kotlin 1.1 or when JetBrain fixes the bug.
-        @Suppress("UNSUPPORTED_FEATURE")
         CordaClassResolver(emptyWhitelistContext).getRegistration(Foo.Bar::class.java)
     }
 
+    @Test(expected = KryoException::class)
+    fun `Unannotated specialised enum does not work`() {
+        CordaClassResolver(emptyWhitelistContext).getRegistration(BadFood.Mud::class.java)
+    }
+
+    @Test
+    fun `Annotation on simple enum works`() {
+        CordaClassResolver(emptyWhitelistContext).getRegistration(Simple.Easy::class.java)
+    }
+
+    @Test(expected = KryoException::class)
+    fun `Unannotated simple enum does not work`() {
+        CordaClassResolver(emptyWhitelistContext).getRegistration(BadSimple.Nasty::class.java)
+    }
+
     @Test
     fun `Annotation on array element works`() {
         val values = arrayOf(Element())
         CordaClassResolver(emptyWhitelistContext).getRegistration(values.javaClass)
     }
 
+    @Test(expected = KryoException::class)
+    fun `Unannotated array elements do not work`() {
+        val values = arrayOf(NotSerializable())
+        CordaClassResolver(emptyWhitelistContext).getRegistration(values.javaClass)
+    }
+
     @Test
     fun `Annotation not needed on abstract class`() {
         CordaClassResolver(emptyWhitelistContext).getRegistration(AbstractClass::class.java)