From 4f033092ed8c9f1b54ff6bed37ec6b87c331e9a9 Mon Sep 17 00:00:00 2001 From: Luca Kellermann Date: Sat, 26 Apr 2025 05:50:34 +0200 Subject: [PATCH 1/3] Defend against attempts to bypass JVM serial proxy Also add serialVersionUID for exception classes. This ensures that unrelated changes in these exception classes don't affect the serialized form. The serialVersionUIDs are from 73803253cdfa10575a374e8bc9e4f977164e1996. After running ./gradlew publishToMavenLocal, they were obtained with the serialver tool [1]: $ serialver -classpath .m2/repository/org/jetbrains/kotlinx/kotlinx-datetime-jvm/0.6.2-SNAPSHOT/kotlinx-datetime-jvm-0.6.2-SNAPSHOT.jar:kotlin-stdlib-2.1.0.jar kotlinx.datetime.LocalDate kotlinx.datetime.LocalDateTime kotlinx.datetime.LocalTime kotlinx.datetime.UtcOffset kotlinx.datetime.DateTimeArithmeticException kotlinx.datetime.IllegalTimeZoneException kotlinx.datetime.DateTimeFormatException kotlinx.datetime.internal.format.parser.ParseException kotlinx.datetime.LocalDate: private static final long serialVersionUID = 7026816023079564263L; kotlinx.datetime.LocalDateTime: private static final long serialVersionUID = -4261744960416354711L; kotlinx.datetime.LocalTime: private static final long serialVersionUID = -352249606036216323L; kotlinx.datetime.UtcOffset: private static final long serialVersionUID = -6636773355667981618L; kotlinx.datetime.DateTimeArithmeticException: private static final long serialVersionUID = -3207806170214997982L; kotlinx.datetime.IllegalTimeZoneException: private static final long serialVersionUID = 1159315966274264801L; kotlinx.datetime.DateTimeFormatException: private static final long serialVersionUID = 4231196759387994100L; kotlinx.datetime.internal.format.parser.ParseException: private static final long serialVersionUID = 5691186997393344103L; [1] https://docs.oracle.com/en/java/javase/21/docs/specs/man/serialver.html --- core/common/src/Exceptions.kt | 12 ++ .../src/internal/format/parser/Parser.kt | 8 +- core/jvm/src/LocalDate.kt | 8 + core/jvm/src/LocalDateTimeJvm.kt | 9 + core/jvm/src/LocalTimeJvm.kt | 8 + core/jvm/src/UtcOffsetJvm.kt | 8 + .../jvm/test/MaliciousJvmSerializationTest.kt | 182 ++++++++++++++++++ 7 files changed, 234 insertions(+), 1 deletion(-) create mode 100644 core/jvm/test/MaliciousJvmSerializationTest.kt diff --git a/core/common/src/Exceptions.kt b/core/common/src/Exceptions.kt index 1f5e93ed..0f43109c 100644 --- a/core/common/src/Exceptions.kt +++ b/core/common/src/Exceptions.kt @@ -13,6 +13,10 @@ public class DateTimeArithmeticException: RuntimeException { public constructor(message: String): super(message) public constructor(cause: Throwable): super(cause) public constructor(message: String, cause: Throwable): super(message, cause) + + private companion object { + private const val serialVersionUID: Long = -3207806170214997982L + } } /** @@ -23,6 +27,10 @@ public class IllegalTimeZoneException: IllegalArgumentException { public constructor(message: String): super(message) public constructor(cause: Throwable): super(cause) public constructor(message: String, cause: Throwable): super(message, cause) + + private companion object { + private const val serialVersionUID: Long = 1159315966274264801L + } } internal class DateTimeFormatException: IllegalArgumentException { @@ -30,4 +38,8 @@ internal class DateTimeFormatException: IllegalArgumentException { constructor(message: String): super(message) constructor(cause: Throwable): super(cause) constructor(message: String, cause: Throwable): super(message, cause) + + private companion object { + private const val serialVersionUID: Long = 4231196759387994100L + } } diff --git a/core/common/src/internal/format/parser/Parser.kt b/core/common/src/internal/format/parser/Parser.kt index 9958e3fb..5e1c3f84 100644 --- a/core/common/src/internal/format/parser/Parser.kt +++ b/core/common/src/internal/format/parser/Parser.kt @@ -209,7 +209,13 @@ internal value class Parser>( ) } -internal class ParseException(errors: List) : Exception(formatError(errors)) +// note that the message of this exception could be anything (even null) after deserialization of a manually constructed +// or corrupted stream (via Java Object Serialization) +internal class ParseException(errors: List) : Exception(formatError(errors)) { + private companion object { + private const val serialVersionUID: Long = 5691186997393344103L + } +} private fun formatError(errors: List): String { if (errors.size == 1) { diff --git a/core/jvm/src/LocalDate.kt b/core/jvm/src/LocalDate.kt index 7e7cda70..9983e2f8 100644 --- a/core/jvm/src/LocalDate.kt +++ b/core/jvm/src/LocalDate.kt @@ -52,6 +52,11 @@ public actual class LocalDate internal constructor( @Suppress("FunctionName") public actual fun Format(block: DateTimeFormatBuilder.WithDate.() -> Unit): DateTimeFormat = LocalDateFormat.build(block) + + // even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a + // stable serialVersionUID means exceptions caused by deserialization of malicious streams will be consistent + // (InvalidClassException vs. InvalidObjectException, see MaliciousJvmSerializationTest) + private const val serialVersionUID = 7026816023079564263L } public actual object Formats { @@ -103,6 +108,9 @@ public actual class LocalDate internal constructor( @JvmName("toEpochDays") internal fun toEpochDaysJvm(): Int = value.toEpochDay().clampToInt() + private fun readObject(ois: java.io.ObjectInputStream): Unit = + throw java.io.InvalidObjectException("kotlinx.datetime.LocalDate must be deserialized via kotlinx.datetime.Ser") + private fun writeReplace(): Any = Ser(Ser.DATE_TAG, this) } diff --git a/core/jvm/src/LocalDateTimeJvm.kt b/core/jvm/src/LocalDateTimeJvm.kt index 235be907..0053c47e 100644 --- a/core/jvm/src/LocalDateTimeJvm.kt +++ b/core/jvm/src/LocalDateTimeJvm.kt @@ -106,12 +106,21 @@ public actual class LocalDateTime internal constructor( @Suppress("FunctionName") public actual fun Format(builder: DateTimeFormatBuilder.WithDateTime.() -> Unit): DateTimeFormat = LocalDateTimeFormat.build(builder) + + // even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a + // stable serialVersionUID means exceptions caused by deserialization of malicious streams will be consistent + // (InvalidClassException vs. InvalidObjectException, see MaliciousJvmSerializationTest) + private const val serialVersionUID: Long = -4261744960416354711L } public actual object Formats { public actual val ISO: DateTimeFormat = ISO_DATETIME } + private fun readObject(ois: java.io.ObjectInputStream): Unit = throw java.io.InvalidObjectException( + "kotlinx.datetime.LocalDateTime must be deserialized via kotlinx.datetime.Ser" + ) + private fun writeReplace(): Any = Ser(Ser.DATE_TIME_TAG, this) } diff --git a/core/jvm/src/LocalTimeJvm.kt b/core/jvm/src/LocalTimeJvm.kt index 98f42011..18f08263 100644 --- a/core/jvm/src/LocalTimeJvm.kt +++ b/core/jvm/src/LocalTimeJvm.kt @@ -85,6 +85,11 @@ public actual class LocalTime internal constructor( @Suppress("FunctionName") public actual fun Format(builder: DateTimeFormatBuilder.WithTime.() -> Unit): DateTimeFormat = LocalTimeFormat.build(builder) + + // even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a + // stable serialVersionUID means exceptions caused by deserialization of malicious streams will be consistent + // (InvalidClassException vs. InvalidObjectException, see MaliciousJvmSerializationTest) + private const val serialVersionUID: Long = -352249606036216323L } public actual object Formats { @@ -92,6 +97,9 @@ public actual class LocalTime internal constructor( } + private fun readObject(ois: java.io.ObjectInputStream): Unit = + throw java.io.InvalidObjectException("kotlinx.datetime.LocalTime must be deserialized via kotlinx.datetime.Ser") + private fun writeReplace(): Any = Ser(Ser.TIME_TAG, this) } diff --git a/core/jvm/src/UtcOffsetJvm.kt b/core/jvm/src/UtcOffsetJvm.kt index 7f9ed703..3228b3f5 100644 --- a/core/jvm/src/UtcOffsetJvm.kt +++ b/core/jvm/src/UtcOffsetJvm.kt @@ -39,6 +39,11 @@ public actual class UtcOffset( @Suppress("FunctionName") public actual fun Format(block: DateTimeFormatBuilder.WithUtcOffset.() -> Unit): DateTimeFormat = UtcOffsetFormat.build(block) + + // even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a + // stable serialVersionUID means exceptions caused by deserialization of malicious streams will be consistent + // (InvalidClassException vs. InvalidObjectException, see MaliciousJvmSerializationTest) + private const val serialVersionUID: Long = -6636773355667981618L } public actual object Formats { @@ -47,6 +52,9 @@ public actual class UtcOffset( public actual val FOUR_DIGITS: DateTimeFormat get() = FOUR_DIGIT_OFFSET } + private fun readObject(ois: java.io.ObjectInputStream): Unit = + throw java.io.InvalidObjectException("kotlinx.datetime.UtcOffset must be deserialized via kotlinx.datetime.Ser") + private fun writeReplace(): Any = Ser(Ser.UTC_OFFSET_TAG, this) } diff --git a/core/jvm/test/MaliciousJvmSerializationTest.kt b/core/jvm/test/MaliciousJvmSerializationTest.kt new file mode 100644 index 00000000..51b410cf --- /dev/null +++ b/core/jvm/test/MaliciousJvmSerializationTest.kt @@ -0,0 +1,182 @@ +/* + * Copyright 2019-2025 JetBrains s.r.o. and contributors. + * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file. + */ + +package kotlinx.datetime + +import kotlinx.datetime.MaliciousJvmSerializationTest.SerDat.Streams +import java.io.ByteArrayInputStream +import java.io.ObjectInputStream +import java.io.Serializable +import kotlin.reflect.KClass +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.fail + +// TODO investigate other null stream (it's different from the one I got) from this comment: +// https://github.com/Kotlin/kotlinx-datetime/pull/373#discussion_r2009789491 +// aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c44617465618443f17dae33e70200014c00097472756556616c75657400154c6a6176612f74696d652f4c6f63616c446174653b787070 + +class MaliciousJvmSerializationTest { + + /** + * This data was generated by running the following Java code (`X` was replaced with the simple name of [clazz]): + * ```java + * package kotlinx.datetime; + * + * import java.io.*; + * import java.util.*; + * + * public class X implements Serializable { + * private final java.time.X = ...; + * + * @Serial + * private static final long serialVersionUID = ...; + * + * public static void main(String[] args) throws IOException { + * var bos = new ByteArrayOutputStream(); + * try (var oos = new ObjectOutputStream(bos)) { + * oos.writeObject(new X()); + * } + * System.out.println(HexFormat.of().formatHex(bos.toByteArray())); + * } + * } + * ``` + */ + private class SerDat( + val clazz: KClass, + /** serialVersionUID had the correct value in the Java code. */ + val withCorrectSVUID: Streams, + /** serialVersionUID had an incorrect value (42) in the Java code. */ + val withSVUID42: Streams, + ) { + class Streams( + /** was set to `null` in the Java code. */ + val delegateNull: String, + /** was set to a valid (non-null) instance in the Java code. */ + val delegateValid: String, + ) + } + + @Suppress("RemoveRedundantQualifierName") + private val data = listOf( + SerDat( + kotlinx.datetime.LocalDate::class, + // generated using "value" as and + // java.time.LocalDate.of(2025, 4, 26) as the valid delegate + withCorrectSVUID = Streams( + delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c44617465618443f17dae33e70200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c446174653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c00007870770703000007e9041a78", + delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c44617465618443f17dae33e70200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c446174653b787070", + ), + withSVUID42 = Streams( + delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c44617465000000000000002a0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c446174653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c00007870770703000007e9041a78", + delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c44617465000000000000002a0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c446174653b787070", + ), + ), + SerDat( + kotlinx.datetime.LocalDateTime::class, + // generated using "value" as and + // java.time.LocalDateTime.of(2025, 4, 26, 11, 18) as the valid delegate + withCorrectSVUID = Streams( + delegateValid = "aced00057372001e6b6f746c696e782e6461746574696d652e4c6f63616c4461746554696d65c4db3d89c7126e690200014c000576616c75657400194c6a6176612f74696d652f4c6f63616c4461746554696d653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c00007870770905000007e9041a0bed78", + delegateNull = "aced00057372001e6b6f746c696e782e6461746574696d652e4c6f63616c4461746554696d65c4db3d89c7126e690200014c000576616c75657400194c6a6176612f74696d652f4c6f63616c4461746554696d653b787070", + ), + withSVUID42 = Streams( + delegateValid = "aced00057372001e6b6f746c696e782e6461746574696d652e4c6f63616c4461746554696d65000000000000002a0200014c000576616c75657400194c6a6176612f74696d652f4c6f63616c4461746554696d653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c00007870770905000007e9041a0bed78", + delegateNull = "aced00057372001e6b6f746c696e782e6461746574696d652e4c6f63616c4461746554696d65000000000000002a0200014c000576616c75657400194c6a6176612f74696d652f4c6f63616c4461746554696d653b787070", + ), + ), + SerDat( + kotlinx.datetime.LocalTime::class, + // generated using "value" as and + // java.time.LocalTime.of(11, 18) as the valid delegate + withCorrectSVUID = Streams( + delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c54696d65fb1c8ed97ff0a5fd0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c54696d653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c000078707703040bed78", + delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c54696d65fb1c8ed97ff0a5fd0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c54696d653b787070", + ), + withSVUID42 = Streams( + delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c54696d65000000000000002a0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c54696d653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c000078707703040bed78", + delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c54696d65000000000000002a0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c54696d653b787070", + ), + ), + SerDat( + kotlinx.datetime.UtcOffset::class, + // generated using "zoneOffset" as and + // java.time.ZoneOffset.UTC as the valid delegate + withCorrectSVUID = Streams( + delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e5574634f6666736574a3e571cbd0a1face0200014c000a7a6f6e654f66667365747400164c6a6176612f74696d652f5a6f6e654f66667365743b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c000078707702080078", + delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e5574634f6666736574a3e571cbd0a1face0200014c000a7a6f6e654f66667365747400164c6a6176612f74696d652f5a6f6e654f66667365743b787070", + ), + withSVUID42 = Streams( + delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e5574634f6666736574000000000000002a0200014c000a7a6f6e654f66667365747400164c6a6176612f74696d652f5a6f6e654f66667365743b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c000078707702080078", + delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e5574634f6666736574000000000000002a0200014c000a7a6f6e654f66667365747400164c6a6176612f74696d652f5a6f6e654f66667365743b787070", + ), + ), + ) + + @OptIn(ExperimentalStdlibApi::class) + private fun deserialize(stream: String): Any? { + val bis = ByteArrayInputStream(stream.hexToByteArray()) + return ObjectInputStream(bis).use { ois -> + ois.readObject() + } + } + + @Test + fun deserializeMaliciousStreams() { + for (d in data) { + val className = d.clazz.qualifiedName!! + testStreamsWithCorrectSVUID(className, d.withCorrectSVUID) + testStreamsWithSVUID42(d.clazz, className, d.withSVUID42) + } + } + + private fun testStreamsWithCorrectSVUID(className: String, streams: Streams) { + val testMessage = "Deserialization of a serial stream that tries to bypass kotlinx.datetime.Ser and has the " + + "correct serialVersionUID for $className should fail" + + val expectedIOEMessage = "$className must be deserialized via kotlinx.datetime.Ser" + + // this would actually create a valid instance, but serialization should always go through the proxy + val ioe1 = assertFailsWith(testMessage) { + deserialize(streams.delegateValid) + } + assertEquals(expectedIOEMessage, ioe1.message) + + // this would create an instance that has null in a non-nullable field (e.g., the field + // kotlinx.datetime.LocalDate.value) + // see https://github.com/Kotlin/kotlinx-datetime/pull/373#discussion_r2008922681 + val ioe2 = assertFailsWith(testMessage) { + deserialize(streams.delegateNull) + } + assertEquals(expectedIOEMessage, ioe2.message) + } + + private fun testStreamsWithSVUID42(clazz: KClass, className: String, streams: Streams) { + val testMessage = "Deserialization of a serial stream that tries to bypass kotlinx.datetime.Ser but has a " + + "wrong serialVersionUID for $className should fail" + + val serialVersionUID = clazz.java + .getDeclaredField("serialVersionUID") + .apply { isAccessible = true } + .get(null) as Long + if (serialVersionUID == 42L) { + fail("This test assumes that the tested classes don't have a serialVersionUID of 42 but $className does.") + } + + val expectedICEMessage = "$className; local class incompatible: stream classdesc serialVersionUID = 42, " + + "local class serialVersionUID = $serialVersionUID" + + val ice1 = assertFailsWith(testMessage) { + deserialize(streams.delegateValid) + } + assertEquals(expectedICEMessage, ice1.message) + + val ice2 = assertFailsWith(testMessage) { + deserialize(streams.delegateNull) + } + assertEquals(expectedICEMessage, ice2.message) + } +} From f051ec03c2c0388d3994a216abc08995a2909350 Mon Sep 17 00:00:00 2001 From: Luca Kellermann Date: Sat, 26 Apr 2025 17:11:54 +0200 Subject: [PATCH 2/3] Ensure test assumptions hold --- .../jvm/test/MaliciousJvmSerializationTest.kt | 111 +++++++++++------- 1 file changed, 70 insertions(+), 41 deletions(-) diff --git a/core/jvm/test/MaliciousJvmSerializationTest.kt b/core/jvm/test/MaliciousJvmSerializationTest.kt index 51b410cf..0f49c387 100644 --- a/core/jvm/test/MaliciousJvmSerializationTest.kt +++ b/core/jvm/test/MaliciousJvmSerializationTest.kt @@ -5,10 +5,11 @@ package kotlinx.datetime -import kotlinx.datetime.MaliciousJvmSerializationTest.SerDat.Streams +import kotlinx.datetime.MaliciousJvmSerializationTest.TestCase.Streams import java.io.ByteArrayInputStream import java.io.ObjectInputStream import java.io.Serializable +import java.lang.reflect.Modifier import kotlin.reflect.KClass import kotlin.test.Test import kotlin.test.assertEquals @@ -22,7 +23,8 @@ import kotlin.test.fail class MaliciousJvmSerializationTest { /** - * This data was generated by running the following Java code (`X` was replaced with the simple name of [clazz]): + * This data was generated by running the following Java code (`X` was replaced with [clazz]`.simpleName`, `Y` with + * [delegate]`::class.qualifiedName` and `z` with [delegateFieldName]): * ```java * package kotlinx.datetime; * @@ -30,7 +32,7 @@ class MaliciousJvmSerializationTest { * import java.util.*; * * public class X implements Serializable { - * private final java.time.X = ...; + * private final Y z = ...; * * @Serial * private static final long serialVersionUID = ...; @@ -45,27 +47,31 @@ class MaliciousJvmSerializationTest { * } * ``` */ - private class SerDat( + private class TestCase( val clazz: KClass, - /** serialVersionUID had the correct value in the Java code. */ + val serialVersionUID: Long, + val delegateFieldName: String, + val delegate: Serializable, + /** serialVersionUID had the correct value ([serialVersionUID]) in the Java code. */ val withCorrectSVUID: Streams, /** serialVersionUID had an incorrect value (42) in the Java code. */ val withSVUID42: Streams, ) { class Streams( - /** was set to `null` in the Java code. */ + /** `z` was set to `null` in the Java code. */ val delegateNull: String, - /** was set to a valid (non-null) instance in the Java code. */ + /** `z` was set to [delegate] in the Java code. */ val delegateValid: String, ) } @Suppress("RemoveRedundantQualifierName") - private val data = listOf( - SerDat( + private val testCases = listOf( + TestCase( kotlinx.datetime.LocalDate::class, - // generated using "value" as and - // java.time.LocalDate.of(2025, 4, 26) as the valid delegate + serialVersionUID = 7026816023079564263L, + delegateFieldName = "value", + delegate = java.time.LocalDate.of(2025, 4, 26), withCorrectSVUID = Streams( delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c44617465618443f17dae33e70200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c446174653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c00007870770703000007e9041a78", delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c44617465618443f17dae33e70200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c446174653b787070", @@ -75,10 +81,11 @@ class MaliciousJvmSerializationTest { delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c44617465000000000000002a0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c446174653b787070", ), ), - SerDat( + TestCase( kotlinx.datetime.LocalDateTime::class, - // generated using "value" as and - // java.time.LocalDateTime.of(2025, 4, 26, 11, 18) as the valid delegate + serialVersionUID = -4261744960416354711L, + delegateFieldName = "value", + delegate = java.time.LocalDateTime.of(2025, 4, 26, 11, 18), withCorrectSVUID = Streams( delegateValid = "aced00057372001e6b6f746c696e782e6461746574696d652e4c6f63616c4461746554696d65c4db3d89c7126e690200014c000576616c75657400194c6a6176612f74696d652f4c6f63616c4461746554696d653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c00007870770905000007e9041a0bed78", delegateNull = "aced00057372001e6b6f746c696e782e6461746574696d652e4c6f63616c4461746554696d65c4db3d89c7126e690200014c000576616c75657400194c6a6176612f74696d652f4c6f63616c4461746554696d653b787070", @@ -88,10 +95,11 @@ class MaliciousJvmSerializationTest { delegateNull = "aced00057372001e6b6f746c696e782e6461746574696d652e4c6f63616c4461746554696d65000000000000002a0200014c000576616c75657400194c6a6176612f74696d652f4c6f63616c4461746554696d653b787070", ), ), - SerDat( + TestCase( kotlinx.datetime.LocalTime::class, - // generated using "value" as and - // java.time.LocalTime.of(11, 18) as the valid delegate + serialVersionUID = -352249606036216323L, + delegateFieldName = "value", + delegate = java.time.LocalTime.of(11, 18), withCorrectSVUID = Streams( delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c54696d65fb1c8ed97ff0a5fd0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c54696d653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c000078707703040bed78", delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c54696d65fb1c8ed97ff0a5fd0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c54696d653b787070", @@ -101,10 +109,11 @@ class MaliciousJvmSerializationTest { delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c54696d65000000000000002a0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c54696d653b787070", ), ), - SerDat( + TestCase( kotlinx.datetime.UtcOffset::class, - // generated using "zoneOffset" as and - // java.time.ZoneOffset.UTC as the valid delegate + serialVersionUID = -6636773355667981618L, + delegateFieldName = "zoneOffset", + delegate = java.time.ZoneOffset.UTC, withCorrectSVUID = Streams( delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e5574634f6666736574a3e571cbd0a1face0200014c000a7a6f6e654f66667365747400164c6a6176612f74696d652f5a6f6e654f66667365743b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c000078707702080078", delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e5574634f6666736574a3e571cbd0a1face0200014c000a7a6f6e654f66667365747400164c6a6176612f74696d652f5a6f6e654f66667365743b787070", @@ -126,21 +135,49 @@ class MaliciousJvmSerializationTest { @Test fun deserializeMaliciousStreams() { - for (d in data) { - val className = d.clazz.qualifiedName!! - testStreamsWithCorrectSVUID(className, d.withCorrectSVUID) - testStreamsWithSVUID42(d.clazz, className, d.withSVUID42) + for (testCase in testCases) { + testCase.ensureAssumptionsHold() + val className = testCase.clazz.qualifiedName!! + testStreamsWithCorrectSVUID(className, testCase.withCorrectSVUID) + testStreamsWithSVUID42(testCase.serialVersionUID, className, testCase.withSVUID42) + } + } + + private fun TestCase.ensureAssumptionsHold() { + val className = clazz.qualifiedName!! + + val actualSerialVersionUID = clazz.java + .getDeclaredField("serialVersionUID") + .apply { isAccessible = true } + .get(null) as Long + if (actualSerialVersionUID == 42L) { + fail("This test assumes that the tested classes don't have a serialVersionUID of 42 but $className does.") + } + if (actualSerialVersionUID != serialVersionUID) { + fail( + "This test assumes that the serialVersionUID of $className is $serialVersionUID but it was " + + "$actualSerialVersionUID." + ) + } + + val field = clazz.java.declaredFields.singleOrNull { !Modifier.isStatic(it.modifiers) } + if (field == null || field.name != delegateFieldName || field.type != delegate.javaClass) { + fail( + "This test assumes that $className has a single instance field named $delegateFieldName of type " + + "${delegate::class.qualifiedName}. The test case for $className should be updated with new " + + "malicious serial streams that represent the changes to $className." + ) } } private fun testStreamsWithCorrectSVUID(className: String, streams: Streams) { - val testMessage = "Deserialization of a serial stream that tries to bypass kotlinx.datetime.Ser and has the " + - "correct serialVersionUID for $className should fail" + val testFailureMessage = "Deserialization of a serial stream that tries to bypass kotlinx.datetime.Ser and " + + "has the correct serialVersionUID for $className should fail" val expectedIOEMessage = "$className must be deserialized via kotlinx.datetime.Ser" // this would actually create a valid instance, but serialization should always go through the proxy - val ioe1 = assertFailsWith(testMessage) { + val ioe1 = assertFailsWith(testFailureMessage) { deserialize(streams.delegateValid) } assertEquals(expectedIOEMessage, ioe1.message) @@ -148,33 +185,25 @@ class MaliciousJvmSerializationTest { // this would create an instance that has null in a non-nullable field (e.g., the field // kotlinx.datetime.LocalDate.value) // see https://github.com/Kotlin/kotlinx-datetime/pull/373#discussion_r2008922681 - val ioe2 = assertFailsWith(testMessage) { + val ioe2 = assertFailsWith(testFailureMessage) { deserialize(streams.delegateNull) } assertEquals(expectedIOEMessage, ioe2.message) } - private fun testStreamsWithSVUID42(clazz: KClass, className: String, streams: Streams) { - val testMessage = "Deserialization of a serial stream that tries to bypass kotlinx.datetime.Ser but has a " + - "wrong serialVersionUID for $className should fail" - - val serialVersionUID = clazz.java - .getDeclaredField("serialVersionUID") - .apply { isAccessible = true } - .get(null) as Long - if (serialVersionUID == 42L) { - fail("This test assumes that the tested classes don't have a serialVersionUID of 42 but $className does.") - } + private fun testStreamsWithSVUID42(serialVersionUID: Long, className: String, streams: Streams) { + val testFailureMessage = "Deserialization of a serial stream that tries to bypass kotlinx.datetime.Ser but " + + "has a wrong serialVersionUID for $className should fail" val expectedICEMessage = "$className; local class incompatible: stream classdesc serialVersionUID = 42, " + "local class serialVersionUID = $serialVersionUID" - val ice1 = assertFailsWith(testMessage) { + val ice1 = assertFailsWith(testFailureMessage) { deserialize(streams.delegateValid) } assertEquals(expectedICEMessage, ice1.message) - val ice2 = assertFailsWith(testMessage) { + val ice2 = assertFailsWith(testFailureMessage) { deserialize(streams.delegateNull) } assertEquals(expectedICEMessage, ice2.message) From 1bb26a942e9f347b17358cfa5ab4f24b3b3bd42c Mon Sep 17 00:00:00 2001 From: Luca Kellermann Date: Sat, 26 Apr 2025 17:47:32 +0200 Subject: [PATCH 3/3] Remove TODO --- core/jvm/test/MaliciousJvmSerializationTest.kt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/jvm/test/MaliciousJvmSerializationTest.kt b/core/jvm/test/MaliciousJvmSerializationTest.kt index 0f49c387..a45d9dbd 100644 --- a/core/jvm/test/MaliciousJvmSerializationTest.kt +++ b/core/jvm/test/MaliciousJvmSerializationTest.kt @@ -16,10 +16,6 @@ import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.fail -// TODO investigate other null stream (it's different from the one I got) from this comment: -// https://github.com/Kotlin/kotlinx-datetime/pull/373#discussion_r2009789491 -// aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c44617465618443f17dae33e70200014c00097472756556616c75657400154c6a6176612f74696d652f4c6f63616c446174653b787070 - class MaliciousJvmSerializationTest { /**