From 5c2df367868ad3542d71fb31c0dafbc66fc43e14 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Mon, 6 Jan 2025 14:42:44 +0000 Subject: [PATCH 1/4] Exclude classes with a writeReplace method from serializability checks --- .../MissingVoidConstructorsOnSerializable.ql | 10 ++++++++ ...ingVoidConstructorsOnSerializable.expected | 1 + ...issingVoidConstructorsOnSerializable.qlref | 1 + .../Test.java | 24 +++++++++++++++++++ 4 files changed, 36 insertions(+) create mode 100644 java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/MissingVoidConstructorsOnSerializable.expected create mode 100644 java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/MissingVoidConstructorsOnSerializable.qlref create mode 100644 java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/Test.java diff --git a/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.ql b/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.ql index 63978a32a367..931b006764e1 100644 --- a/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.ql +++ b/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.ql @@ -24,6 +24,16 @@ where c.hasNoParameters() and not c.isPrivate() ) and + // Assume if an object replaces itself prior to serialization, + // then it is unlikely to be directly deserialized. + // That means it won't need to comply with default serialization rules, + // such as non-serializable super-classes having a no-argument constructor. + not exists(Method m | + m = serial.getAMethod() and + m.hasName("writeReplace") and + m.getReturnType() instanceof TypeObject and + m.hasNoParameters() + ) and serial.fromSource() select serial, "This class is serializable, but its non-serializable " + diff --git a/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/MissingVoidConstructorsOnSerializable.expected b/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/MissingVoidConstructorsOnSerializable.expected new file mode 100644 index 000000000000..ab7aa578dd42 --- /dev/null +++ b/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/MissingVoidConstructorsOnSerializable.expected @@ -0,0 +1 @@ +| Test.java:12:7:12:7 | A | This class is serializable, but its non-serializable super-class $@ does not declare a no-argument constructor. | Test.java:4:7:4:20 | NonSerialzable | NonSerialzable | diff --git a/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/MissingVoidConstructorsOnSerializable.qlref b/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/MissingVoidConstructorsOnSerializable.qlref new file mode 100644 index 000000000000..26bbcf24bbb8 --- /dev/null +++ b/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/MissingVoidConstructorsOnSerializable.qlref @@ -0,0 +1 @@ +Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.ql diff --git a/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/Test.java b/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/Test.java new file mode 100644 index 000000000000..1787fe850153 --- /dev/null +++ b/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/Test.java @@ -0,0 +1,24 @@ +import java.io.ObjectStreamException; +import java.io.Serializable; + +class NonSerialzable { + + // Has no default constructor + public NonSerialzable(int x) { } + +} + +// BAD: Serializable but its parent cannot be instantiated +class A extends NonSerialzable implements Serializable { + public A() { super(1); } +} + +// GOOD: writeReplaces itself, so unlikely to be deserialized +// according to default rules. +class B extends NonSerialzable implements Serializable { + public B() { super(2); } + + public Object writeReplace() throws ObjectStreamException { + return null; + } +} From d0eab598b18067b2e16553ec1e7b865062f47c43 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Mon, 6 Jan 2025 14:44:12 +0000 Subject: [PATCH 2/4] Change note --- .../src/change-notes/2025-01-06-write-replace-serializable.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 java/ql/src/change-notes/2025-01-06-write-replace-serializable.md diff --git a/java/ql/src/change-notes/2025-01-06-write-replace-serializable.md b/java/ql/src/change-notes/2025-01-06-write-replace-serializable.md new file mode 100644 index 000000000000..9eceda87acec --- /dev/null +++ b/java/ql/src/change-notes/2025-01-06-write-replace-serializable.md @@ -0,0 +1,4 @@ +--- +category: fix +--- +* Classes that define a `writeReplace` method are no longer flagged by the `java/missing-no-arg-constructor-on-serializable` query on the assumption they are unlikely to be deserialized using the default algorithm. From 03c6529961f94b90c0c76b2e74048d3052ac982c Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Mon, 6 Jan 2025 22:46:22 +0100 Subject: [PATCH 3/4] Spelling --- .../MissingVoidConstructorsOnSerializable.expected | 2 +- .../MissingVoidConstructorsOnSerializable/Test.java | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/MissingVoidConstructorsOnSerializable.expected b/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/MissingVoidConstructorsOnSerializable.expected index ab7aa578dd42..02c3434f1573 100644 --- a/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/MissingVoidConstructorsOnSerializable.expected +++ b/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/MissingVoidConstructorsOnSerializable.expected @@ -1 +1 @@ -| Test.java:12:7:12:7 | A | This class is serializable, but its non-serializable super-class $@ does not declare a no-argument constructor. | Test.java:4:7:4:20 | NonSerialzable | NonSerialzable | +| Test.java:12:7:12:7 | A | This class is serializable, but its non-serializable super-class $@ does not declare a no-argument constructor. | Test.java:4:7:4:21 | NonSerializable | NonSerializable | diff --git a/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/Test.java b/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/Test.java index 1787fe850153..f20f5ac8f495 100644 --- a/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/Test.java +++ b/java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/Test.java @@ -1,21 +1,21 @@ import java.io.ObjectStreamException; import java.io.Serializable; -class NonSerialzable { +class NonSerializable { // Has no default constructor - public NonSerialzable(int x) { } + public NonSerializable(int x) { } } // BAD: Serializable but its parent cannot be instantiated -class A extends NonSerialzable implements Serializable { +class A extends NonSerializable implements Serializable { public A() { super(1); } } // GOOD: writeReplaces itself, so unlikely to be deserialized // according to default rules. -class B extends NonSerialzable implements Serializable { +class B extends NonSerializable implements Serializable { public B() { super(2); } public Object writeReplace() throws ObjectStreamException { From dd0012edcb9f05bd2fd5a963b5ff79cd9e4e25a8 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Mon, 6 Jan 2025 23:28:02 +0100 Subject: [PATCH 4/4] ASCII --- .../Serialization/MissingVoidConstructorsOnSerializable.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.ql b/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.ql index 931b006764e1..651908b0b865 100644 --- a/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.ql +++ b/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.ql @@ -25,7 +25,7 @@ where not c.isPrivate() ) and // Assume if an object replaces itself prior to serialization, - // then it is unlikely to be directly deserialized. + // then it is unlikely to be directly deserialized. // That means it won't need to comply with default serialization rules, // such as non-serializable super-classes having a no-argument constructor. not exists(Method m |