From 4f033092ed8c9f1b54ff6bed37ec6b87c331e9a9 Mon Sep 17 00:00:00 2001 From: Luca Kellermann Date: Sat, 26 Apr 2025 05:50:34 +0200 Subject: [PATCH 01/10] 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 1f5e93ed2..0f43109c2 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 9958e3fb9..5e1c3f84d 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 7e7cda702..9983e2f86 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 235be9071..0053c47ec 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 98f42011b..18f08263a 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 7f9ed7031..3228b3f5d 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 000000000..51b410cf6 --- /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 02/10] 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 51b410cf6..0f49c3877 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 03/10] 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 0f49c3877..a45d9dbdd 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 { /** From b4ba14bdd12879fcd684f0111822d265660dfd9f Mon Sep 17 00:00:00 2001 From: Luca Kellermann Date: Tue, 27 May 2025 22:51:19 +0200 Subject: [PATCH 04/10] Change package of MaliciousJvmSerializationTest --- core/jvm/test/MaliciousJvmSerializationTest.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/jvm/test/MaliciousJvmSerializationTest.kt b/core/jvm/test/MaliciousJvmSerializationTest.kt index a45d9dbdd..310dd1575 100644 --- a/core/jvm/test/MaliciousJvmSerializationTest.kt +++ b/core/jvm/test/MaliciousJvmSerializationTest.kt @@ -3,9 +3,9 @@ * 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 +package kotlinx.datetime.test -import kotlinx.datetime.MaliciousJvmSerializationTest.TestCase.Streams +import kotlinx.datetime.test.MaliciousJvmSerializationTest.TestCase.Streams import java.io.ByteArrayInputStream import java.io.ObjectInputStream import java.io.Serializable @@ -61,7 +61,6 @@ class MaliciousJvmSerializationTest { ) } - @Suppress("RemoveRedundantQualifierName") private val testCases = listOf( TestCase( kotlinx.datetime.LocalDate::class, From 39f5dc41e27020baa4a262ddffc2c1eb1f96c6b8 Mon Sep 17 00:00:00 2001 From: Luca Kellermann Date: Sun, 1 Jun 2025 02:05:25 +0200 Subject: [PATCH 05/10] Use ObjectStreamClass to check assumptions --- core/jvm/test/MaliciousJvmSerializationTest.kt | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/core/jvm/test/MaliciousJvmSerializationTest.kt b/core/jvm/test/MaliciousJvmSerializationTest.kt index 310dd1575..ffee1d6f2 100644 --- a/core/jvm/test/MaliciousJvmSerializationTest.kt +++ b/core/jvm/test/MaliciousJvmSerializationTest.kt @@ -8,8 +8,8 @@ package kotlinx.datetime.test import kotlinx.datetime.test.MaliciousJvmSerializationTest.TestCase.Streams import java.io.ByteArrayInputStream import java.io.ObjectInputStream +import java.io.ObjectStreamClass import java.io.Serializable -import java.lang.reflect.Modifier import kotlin.reflect.KClass import kotlin.test.Test import kotlin.test.assertEquals @@ -140,11 +140,9 @@ class MaliciousJvmSerializationTest { private fun TestCase.ensureAssumptionsHold() { val className = clazz.qualifiedName!! + val objectStreamClass = ObjectStreamClass.lookup(clazz.java) - val actualSerialVersionUID = clazz.java - .getDeclaredField("serialVersionUID") - .apply { isAccessible = true } - .get(null) as Long + val actualSerialVersionUID = objectStreamClass.serialVersionUID if (actualSerialVersionUID == 42L) { fail("This test assumes that the tested classes don't have a serialVersionUID of 42 but $className does.") } @@ -155,11 +153,11 @@ class MaliciousJvmSerializationTest { ) } - val field = clazz.java.declaredFields.singleOrNull { !Modifier.isStatic(it.modifiers) } + val field = objectStreamClass.fields.singleOrNull() 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 " + + "This test assumes that $className has a single serializable 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." ) } From 8811dcc0d72267a7b0cdd36a9d201f23db28e3c3 Mon Sep 17 00:00:00 2001 From: Luca Kellermann Date: Sun, 1 Jun 2025 03:24:06 +0200 Subject: [PATCH 06/10] Update comments for serialVersionUID --- core/jvm/src/LocalDate.kt | 5 ++--- core/jvm/src/LocalDateTimeJvm.kt | 5 ++--- core/jvm/src/LocalTimeJvm.kt | 5 ++--- core/jvm/src/UtcOffsetJvm.kt | 5 ++--- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/core/jvm/src/LocalDate.kt b/core/jvm/src/LocalDate.kt index dd8efbd37..6ec88b3aa 100644 --- a/core/jvm/src/LocalDate.kt +++ b/core/jvm/src/LocalDate.kt @@ -53,9 +53,8 @@ public actual class LocalDate internal constructor( 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) + // Even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a + // stable serialVersionUID is useful for testing, see MaliciousJvmSerializationTest. private const val serialVersionUID = 7026816023079564263L } diff --git a/core/jvm/src/LocalDateTimeJvm.kt b/core/jvm/src/LocalDateTimeJvm.kt index 4777b4974..f785f142c 100644 --- a/core/jvm/src/LocalDateTimeJvm.kt +++ b/core/jvm/src/LocalDateTimeJvm.kt @@ -107,9 +107,8 @@ public actual class LocalDateTime internal constructor( 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) + // Even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a + // stable serialVersionUID is useful for testing, see MaliciousJvmSerializationTest. private const val serialVersionUID: Long = -4261744960416354711L } diff --git a/core/jvm/src/LocalTimeJvm.kt b/core/jvm/src/LocalTimeJvm.kt index 9ad042537..3a5d26b3a 100644 --- a/core/jvm/src/LocalTimeJvm.kt +++ b/core/jvm/src/LocalTimeJvm.kt @@ -86,9 +86,8 @@ public actual class LocalTime internal constructor( 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) + // Even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a + // stable serialVersionUID is useful for testing, see MaliciousJvmSerializationTest. private const val serialVersionUID: Long = -352249606036216323L } diff --git a/core/jvm/src/UtcOffsetJvm.kt b/core/jvm/src/UtcOffsetJvm.kt index 1b9d87266..cf35ee13a 100644 --- a/core/jvm/src/UtcOffsetJvm.kt +++ b/core/jvm/src/UtcOffsetJvm.kt @@ -41,9 +41,8 @@ public actual class UtcOffset( 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) + // Even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a + // stable serialVersionUID is useful for testing, see MaliciousJvmSerializationTest. private const val serialVersionUID: Long = -6636773355667981618L } From 68e8737ec04485d1d95a5d905d74fb10bbbc66dc Mon Sep 17 00:00:00 2001 From: Luca Kellermann Date: Tue, 3 Jun 2025 19:04:56 +0200 Subject: [PATCH 07/10] Use serialVersionUID 0L for simplicity --- core/jvm/src/LocalDate.kt | 2 +- core/jvm/src/LocalDateTimeJvm.kt | 2 +- core/jvm/src/LocalTimeJvm.kt | 2 +- core/jvm/src/UtcOffsetJvm.kt | 2 +- .../jvm/test/MaliciousJvmSerializationTest.kt | 51 ++++++++----------- 5 files changed, 24 insertions(+), 35 deletions(-) diff --git a/core/jvm/src/LocalDate.kt b/core/jvm/src/LocalDate.kt index 6ec88b3aa..23066f8b2 100644 --- a/core/jvm/src/LocalDate.kt +++ b/core/jvm/src/LocalDate.kt @@ -55,7 +55,7 @@ public actual class LocalDate internal constructor( // Even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a // stable serialVersionUID is useful for testing, see MaliciousJvmSerializationTest. - private const val serialVersionUID = 7026816023079564263L + private const val serialVersionUID = 0L } public actual object Formats { diff --git a/core/jvm/src/LocalDateTimeJvm.kt b/core/jvm/src/LocalDateTimeJvm.kt index f785f142c..adf0c6ac6 100644 --- a/core/jvm/src/LocalDateTimeJvm.kt +++ b/core/jvm/src/LocalDateTimeJvm.kt @@ -109,7 +109,7 @@ public actual class LocalDateTime internal constructor( // Even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a // stable serialVersionUID is useful for testing, see MaliciousJvmSerializationTest. - private const val serialVersionUID: Long = -4261744960416354711L + private const val serialVersionUID: Long = 0L } public actual object Formats { diff --git a/core/jvm/src/LocalTimeJvm.kt b/core/jvm/src/LocalTimeJvm.kt index 3a5d26b3a..fbd548385 100644 --- a/core/jvm/src/LocalTimeJvm.kt +++ b/core/jvm/src/LocalTimeJvm.kt @@ -88,7 +88,7 @@ public actual class LocalTime internal constructor( // Even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a // stable serialVersionUID is useful for testing, see MaliciousJvmSerializationTest. - private const val serialVersionUID: Long = -352249606036216323L + private const val serialVersionUID: Long = 0L } public actual object Formats { diff --git a/core/jvm/src/UtcOffsetJvm.kt b/core/jvm/src/UtcOffsetJvm.kt index cf35ee13a..c6ed32db1 100644 --- a/core/jvm/src/UtcOffsetJvm.kt +++ b/core/jvm/src/UtcOffsetJvm.kt @@ -43,7 +43,7 @@ public actual class UtcOffset( // Even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a // stable serialVersionUID is useful for testing, see MaliciousJvmSerializationTest. - private const val serialVersionUID: Long = -6636773355667981618L + private const val serialVersionUID: Long = 0L } public actual object Formats { diff --git a/core/jvm/test/MaliciousJvmSerializationTest.kt b/core/jvm/test/MaliciousJvmSerializationTest.kt index ffee1d6f2..abbca2923 100644 --- a/core/jvm/test/MaliciousJvmSerializationTest.kt +++ b/core/jvm/test/MaliciousJvmSerializationTest.kt @@ -45,13 +45,12 @@ class MaliciousJvmSerializationTest { */ private class TestCase( val clazz: KClass, - val serialVersionUID: Long, val delegateFieldName: String, val delegate: Serializable, - /** serialVersionUID had the correct value ([serialVersionUID]) in the Java code. */ + /** `serialVersionUID` was set to the correct value (`0L`) in the Java code. */ val withCorrectSVUID: Streams, - /** serialVersionUID had an incorrect value (42) in the Java code. */ - val withSVUID42: Streams, + /** `serialVersionUID` was set to an incorrect value (`42L`) in the Java code. */ + val withIncorrectSVUID: Streams, ) { class Streams( /** `z` was set to `null` in the Java code. */ @@ -64,56 +63,52 @@ class MaliciousJvmSerializationTest { private val testCases = listOf( TestCase( kotlinx.datetime.LocalDate::class, - serialVersionUID = 7026816023079564263L, delegateFieldName = "value", delegate = java.time.LocalDate.of(2025, 4, 26), withCorrectSVUID = Streams( - delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c44617465618443f17dae33e70200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c446174653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c00007870770703000007e9041a78", - delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c44617465618443f17dae33e70200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c446174653b787070", + delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c4461746500000000000000000200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c446174653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c00007870770703000007e9041a78", + delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c4461746500000000000000000200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c446174653b787070", ), - withSVUID42 = Streams( + withIncorrectSVUID = Streams( delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c44617465000000000000002a0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c446174653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c00007870770703000007e9041a78", delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c44617465000000000000002a0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c446174653b787070", ), ), TestCase( kotlinx.datetime.LocalDateTime::class, - serialVersionUID = -4261744960416354711L, delegateFieldName = "value", delegate = java.time.LocalDateTime.of(2025, 4, 26, 11, 18), withCorrectSVUID = Streams( - delegateValid = "aced00057372001e6b6f746c696e782e6461746574696d652e4c6f63616c4461746554696d65c4db3d89c7126e690200014c000576616c75657400194c6a6176612f74696d652f4c6f63616c4461746554696d653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c00007870770905000007e9041a0bed78", - delegateNull = "aced00057372001e6b6f746c696e782e6461746574696d652e4c6f63616c4461746554696d65c4db3d89c7126e690200014c000576616c75657400194c6a6176612f74696d652f4c6f63616c4461746554696d653b787070", + delegateValid = "aced00057372001e6b6f746c696e782e6461746574696d652e4c6f63616c4461746554696d6500000000000000000200014c000576616c75657400194c6a6176612f74696d652f4c6f63616c4461746554696d653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c00007870770905000007e9041a0bed78", + delegateNull = "aced00057372001e6b6f746c696e782e6461746574696d652e4c6f63616c4461746554696d6500000000000000000200014c000576616c75657400194c6a6176612f74696d652f4c6f63616c4461746554696d653b787070", ), - withSVUID42 = Streams( + withIncorrectSVUID = Streams( delegateValid = "aced00057372001e6b6f746c696e782e6461746574696d652e4c6f63616c4461746554696d65000000000000002a0200014c000576616c75657400194c6a6176612f74696d652f4c6f63616c4461746554696d653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c00007870770905000007e9041a0bed78", delegateNull = "aced00057372001e6b6f746c696e782e6461746574696d652e4c6f63616c4461746554696d65000000000000002a0200014c000576616c75657400194c6a6176612f74696d652f4c6f63616c4461746554696d653b787070", ), ), TestCase( kotlinx.datetime.LocalTime::class, - serialVersionUID = -352249606036216323L, delegateFieldName = "value", delegate = java.time.LocalTime.of(11, 18), withCorrectSVUID = Streams( - delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c54696d65fb1c8ed97ff0a5fd0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c54696d653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c000078707703040bed78", - delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c54696d65fb1c8ed97ff0a5fd0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c54696d653b787070", + delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c54696d6500000000000000000200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c54696d653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c000078707703040bed78", + delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c54696d6500000000000000000200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c54696d653b787070", ), - withSVUID42 = Streams( + withIncorrectSVUID = Streams( delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c54696d65000000000000002a0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c54696d653b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c000078707703040bed78", delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c54696d65000000000000002a0200014c000576616c75657400154c6a6176612f74696d652f4c6f63616c54696d653b787070", ), ), TestCase( kotlinx.datetime.UtcOffset::class, - serialVersionUID = -6636773355667981618L, delegateFieldName = "zoneOffset", delegate = java.time.ZoneOffset.UTC, withCorrectSVUID = Streams( - delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e5574634f6666736574a3e571cbd0a1face0200014c000a7a6f6e654f66667365747400164c6a6176612f74696d652f5a6f6e654f66667365743b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c000078707702080078", - delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e5574634f6666736574a3e571cbd0a1face0200014c000a7a6f6e654f66667365747400164c6a6176612f74696d652f5a6f6e654f66667365743b787070", + delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e5574634f666673657400000000000000000200014c000a7a6f6e654f66667365747400164c6a6176612f74696d652f5a6f6e654f66667365743b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c000078707702080078", + delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e5574634f666673657400000000000000000200014c000a7a6f6e654f66667365747400164c6a6176612f74696d652f5a6f6e654f66667365743b787070", ), - withSVUID42 = Streams( + withIncorrectSVUID = Streams( delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e5574634f6666736574000000000000002a0200014c000a7a6f6e654f66667365747400164c6a6176612f74696d652f5a6f6e654f66667365743b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c000078707702080078", delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e5574634f6666736574000000000000002a0200014c000a7a6f6e654f66667365747400164c6a6176612f74696d652f5a6f6e654f66667365743b787070", ), @@ -134,7 +129,7 @@ class MaliciousJvmSerializationTest { testCase.ensureAssumptionsHold() val className = testCase.clazz.qualifiedName!! testStreamsWithCorrectSVUID(className, testCase.withCorrectSVUID) - testStreamsWithSVUID42(testCase.serialVersionUID, className, testCase.withSVUID42) + testStreamsWithIncorrectSVUID(className, testCase.withIncorrectSVUID) } } @@ -143,14 +138,8 @@ class MaliciousJvmSerializationTest { val objectStreamClass = ObjectStreamClass.lookup(clazz.java) val actualSerialVersionUID = objectStreamClass.serialVersionUID - 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." - ) + if (actualSerialVersionUID != 0L) { + fail("This test assumes that the serialVersionUID of $className is 0, but it was $actualSerialVersionUID.") } val field = objectStreamClass.fields.singleOrNull() @@ -184,12 +173,12 @@ class MaliciousJvmSerializationTest { assertEquals(expectedIOEMessage, ioe2.message) } - private fun testStreamsWithSVUID42(serialVersionUID: Long, className: String, streams: Streams) { + private fun testStreamsWithIncorrectSVUID(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" + "local class serialVersionUID = 0" val ice1 = assertFailsWith(testFailureMessage) { deserialize(streams.delegateValid) From c28dee424cd2fa9651255a0841aa5826eb119e2e Mon Sep 17 00:00:00 2001 From: Luca Kellermann Date: Tue, 3 Jun 2025 20:51:32 +0200 Subject: [PATCH 08/10] Revert serialVersionUID for exception classes --- core/common/src/Exceptions.kt | 12 ------------ core/common/src/internal/format/parser/Parser.kt | 8 +------- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/core/common/src/Exceptions.kt b/core/common/src/Exceptions.kt index 0f43109c2..1f5e93ed2 100644 --- a/core/common/src/Exceptions.kt +++ b/core/common/src/Exceptions.kt @@ -13,10 +13,6 @@ 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 - } } /** @@ -27,10 +23,6 @@ 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 { @@ -38,8 +30,4 @@ 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 5e1c3f84d..9958e3fb9 100644 --- a/core/common/src/internal/format/parser/Parser.kt +++ b/core/common/src/internal/format/parser/Parser.kt @@ -209,13 +209,7 @@ internal value class Parser>( ) } -// 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 - } -} +internal class ParseException(errors: List) : Exception(formatError(errors)) private fun formatError(errors: List): String { if (errors.size == 1) { From 0c20353a24c347474846344074b8ae4360a1e470 Mon Sep 17 00:00:00 2001 From: Luca Kellermann Date: Tue, 3 Jun 2025 20:57:49 +0200 Subject: [PATCH 09/10] Explicit type for LocalDate.serialVersionUID --- core/jvm/src/LocalDate.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/jvm/src/LocalDate.kt b/core/jvm/src/LocalDate.kt index 23066f8b2..0a082a15a 100644 --- a/core/jvm/src/LocalDate.kt +++ b/core/jvm/src/LocalDate.kt @@ -55,7 +55,7 @@ public actual class LocalDate internal constructor( // Even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a // stable serialVersionUID is useful for testing, see MaliciousJvmSerializationTest. - private const val serialVersionUID = 0L + private const val serialVersionUID: Long = 0L } public actual object Formats { From a6819851359986736b626fbce82ad824e8f8a42f Mon Sep 17 00:00:00 2001 From: Luca Kellermann Date: Sat, 21 Jun 2025 09:18:14 +0200 Subject: [PATCH 10/10] Include YearMonth --- core/jvm/src/YearMonthJvm.kt | 7 +++++++ core/jvm/test/MaliciousJvmSerializationTest.kt | 17 +++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/core/jvm/src/YearMonthJvm.kt b/core/jvm/src/YearMonthJvm.kt index 560399562..c3999a21d 100644 --- a/core/jvm/src/YearMonthJvm.kt +++ b/core/jvm/src/YearMonthJvm.kt @@ -55,6 +55,10 @@ public actual class YearMonth internal constructor( @Suppress("FunctionName") public actual fun Format(block: DateTimeFormatBuilder.WithYearMonth.() -> Unit): DateTimeFormat = YearMonthFormat.build(block) + + // Even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a + // stable serialVersionUID is useful for testing, see MaliciousJvmSerializationTest. + private const val serialVersionUID: Long = 0L } public actual object Formats { @@ -73,6 +77,9 @@ public actual class YearMonth internal constructor( override fun hashCode(): Int = value.hashCode() + private fun readObject(ois: java.io.ObjectInputStream): Unit = + throw java.io.InvalidObjectException("kotlinx.datetime.YearMonth must be deserialized via kotlinx.datetime.Ser") + private fun writeReplace(): Any = Ser(Ser.YEAR_MONTH_TAG, this) } diff --git a/core/jvm/test/MaliciousJvmSerializationTest.kt b/core/jvm/test/MaliciousJvmSerializationTest.kt index abbca2923..3ee901dcb 100644 --- a/core/jvm/test/MaliciousJvmSerializationTest.kt +++ b/core/jvm/test/MaliciousJvmSerializationTest.kt @@ -53,10 +53,10 @@ class MaliciousJvmSerializationTest { val withIncorrectSVUID: Streams, ) { class Streams( - /** `z` was set to `null` in the Java code. */ - val delegateNull: String, /** `z` was set to [delegate] in the Java code. */ val delegateValid: String, + /** `z` was set to `null` in the Java code. */ + val delegateNull: String, ) } @@ -113,6 +113,19 @@ class MaliciousJvmSerializationTest { delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e5574634f6666736574000000000000002a0200014c000a7a6f6e654f66667365747400164c6a6176612f74696d652f5a6f6e654f66667365743b787070", ), ), + TestCase( + kotlinx.datetime.YearMonth::class, + delegateFieldName = "value", + delegate = java.time.YearMonth.of(2025, 4), + withCorrectSVUID = Streams( + delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e596561724d6f6e746800000000000000000200014c000576616c75657400154c6a6176612f74696d652f596561724d6f6e74683b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c0000787077060c000007e90478", + delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e596561724d6f6e746800000000000000000200014c000576616c75657400154c6a6176612f74696d652f596561724d6f6e74683b787070", + ), + withIncorrectSVUID = Streams( + delegateValid = "aced00057372001a6b6f746c696e782e6461746574696d652e596561724d6f6e7468000000000000002a0200014c000576616c75657400154c6a6176612f74696d652f596561724d6f6e74683b78707372000d6a6176612e74696d652e536572955d84ba1b2248b20c0000787077060c000007e90478", + delegateNull = "aced00057372001a6b6f746c696e782e6461746574696d652e596561724d6f6e7468000000000000002a0200014c000576616c75657400154c6a6176612f74696d652f596561724d6f6e74683b787070", + ), + ), ) @OptIn(ExperimentalStdlibApi::class)