From: Robert Varga <robert.varga@pantheon.tech>
Date: Wed, 10 Apr 2024 13:50:16 +0000 (+0200)
Subject: Defeat potential singleton pollution attacks
X-Git-Tag: v13.0.3~3
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=5d2a3a0e46e802de92e96b2e28bc29a7ae43a2b0;p=yangtools.git

Defeat potential singleton pollution attacks

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

diff --git a/common/yang-common/src/main/java/org/opendaylight/yangtools/yang/common/Empty.java b/common/yang-common/src/main/java/org/opendaylight/yangtools/yang/common/Empty.java
index 88edd68450..8cb57ec3fa 100644
--- a/common/yang-common/src/main/java/org/opendaylight/yangtools/yang/common/Empty.java
+++ b/common/yang-common/src/main/java/org/opendaylight/yangtools/yang/common/Empty.java
@@ -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