Incorrect Rule: java/missing-no-arg-constructor-on-serializable

[ben-manes]

codeql/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.ql

Lines 2 to 5 in c95f8d7

	* @name Serializable but no void constructor
	* @description A non-serializable, immediate superclass of a serializable class that does not
	* itself declare an accessible, no-argument constructor causes deserialization to
	* fail.

This rule does not take into consideration the Serialization Proxy pattern. This approach is recommended in Effective Java, a reference this rule's explanation points to. The proxy pattern is both a simplification and protection from security threats by reconstructing the object from the data elements, using readResolve to replace the output with a new instance, rather than trying to rehydrate the object directly.

While ideally this rule would understand this pattern, it is okay if a false positive yet it recommends that as the solution. A less aware developer might remove this security protection to comply or not be nudged to implement that better approach if a true positive.

[jketema]

Hi @ben-manes,

Thanks for your observation. I've relayed this to the CodeQL Java engineering team.

[smowton]

As of CodeQL 2.20.2 we will no longer flag classes that define writeReplace and so which are assumed to perhaps never be exposed to the default deserialization algorithm and its requirement for an accessible parent-class constructor.

[ben-manes]

Thanks! I’d recommend also updating the rule’s suggested fix to inform the user of the serialization proxy pattern for security reasons. While thankfully native serialization is rare, when required due to legacy compatibility, users should be hinted towards secure practices.

[smowton]

Done in #18480