Exclude classes with a writeReplace method from serializability checks

Author: smowton

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 " +

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 |

java/ql/test/query-tests/MissingVoidConstructorsOnSerializable/MissingVoidConstructorsOnSerializable.qlref

@@ -0,0 +1 @@
+Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.ql

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;
+  }
+}