Defeat potential singleton pollution attacks 48/111348/1
authorRobert Varga <robert.varga@pantheon.tech>
Wed, 10 Apr 2024 13:50:16 +0000 (15:50 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Wed, 10 Apr 2024 13:58:57 +0000 (15:58 +0200)
yang.common.Empty is a Serializable, which has no state beside its class
hierarchy. It therefore lends itself to having a singleton value object
-- except SpotBugs flags this as a violation MSC07-J rule in SEI CERT
Oracle Coding Standard for Java.

Digging into this, the readResolve() guard could be defeated via
hand-crafted serialization -- and thus could in theory have more than
once instance. Doing so would allow to have two Empty instances which
have the same Class, but do not compare as equal.

This patch erases that possibility, fixing a possibility of the error.

Also update documentation a bit to remove the word 'singleton', so users
are not tempted to use '==' for comparison -- which is already a bad
idea anyway.

Change-Id: Ibccd2909790e7a46d002d80f7b36280ff27fb724
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
common/yang-common/src/main/java/org/opendaylight/yangtools/yang/common/Empty.java

index 88edd68450416b45669545b9e315e1f1d5c32c47..8cb57ec3fae478b5d881392b03f145db1e864087 100644 (file)
@@ -17,7 +17,7 @@ import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.yangtools.concepts.Immutable;
 
 /**
- * Dedicated singleton type for YANG's {@code type empty} value.
+ * Dedicated type for YANG's {@code type empty} value.
  */
 @NonNullByDefault
 public final class Empty implements Immutable, Serializable {
@@ -65,7 +65,9 @@ public final class Empty implements Immutable, Serializable {
 
     @Override
     public boolean equals(final @Nullable Object obj) {
-        return this == obj;
+        // Note: this is nominally a singleton, but due to it being Serializable multiple instances might be created
+        //       via hand-crafted serialization streams. We therefore do not rely on '==' but on 'instanceof' check.
+        return obj instanceof Empty;
     }
 
     @Override