Improved: Improve ObjectInputStream denyList (OFBIZ-12221)

JacquesLeRoux · JacquesLeRoux · commit 3f975780cfab · 2021-04-07T08:51:36.000+02:00
In SafeObjectInputStream.properties
  Renames listOfSafeObjectsForInputStream to allowList and fixes it
  Introduces a denyList

Adapts SafeObjectInputStream class to new denyList
diff --git a/framework/base/config/SafeObjectInputStream.properties b/framework/base/config/SafeObjectInputStream.properties
@@ -17,13 +17,18 @@
 # under the License.
 ###############################################################################
 
-# Because of OFBIZ-10837 - Improve ObjectInputStream class. 
-# If you encounter a related issue (object not in the allowlist), 
-# you must provide a complete list of objects to pass to ObjectInputStream 
-# through ListOfSafeObjectsForInputStream property
-# As an example, the a complete list of objects  used by OFBiz OOTB is here.
+# Because of OFBIZ-10837 "Improve ObjectInputStream class."
+# If you encounter a related issue (object not in the allowList),
+# you must provide a complete list of objects to pass to ObjectInputStream through allowList property
+# As an example, the a complete list of objects used by OFBiz OOTB is here in allowList.
 # You will need to add your objects/classes to this list.
-# OFBiz committers: don't forget to add newobjects in SafeObjectInputStream class too (as default there).
 
+# OFBiz committers:
+#     . don't forget to add new objects in SafeObjectInputStream class too (as default there).
+#     . "foo" and "SerializationInjector" are used in OFBiz tests
 
-listOfSafeObjectsForInputStream=byte\\\\[\\\\], foo, SerializationInjector, \\\\[Z,\\\\[B,\\\\[S,\\\\[I,\\\\[J,\\\\[F,\\\\[D,\\\\[C, java..*, sun.util.calendar..*, org.apache.ofbiz..*, org.codehaus.groovy.runtime.GStringImpl, groovy.lang.GString
+allowList=byte\\[\\], foo, SerializationInjector, \\[Z,\\[B,\\[S,\\[I,\\[J,\\[F,\\[D,\\[C, java..*, sun.util.calendar..*, org.apache.ofbiz..*, org.codehaus.groovy.runtime.GStringImpl, groovy.lang.GString
+
+#-- List of strings rejected for serialisation
+#-- The same comments than for allowList apply to denyList
+denyList=rmi, <
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/SafeObjectInputStream.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/SafeObjectInputStream.java
@@ -42,6 +42,7 @@ public final class SafeObjectInputStream extends ObjectInputStream {
             "\\[Z", "\\[B", "\\[S", "\\[I", "\\[J", "\\[F", "\\[D", "\\[C",
             "java..*", "sun.util.calendar..*", "org.apache.ofbiz..*",
             "org.codehaus.groovy.runtime.GStringImpl", "groovy.lang.GString"};
+    private static final String[] DEFAULT_DENYLIST = { "rmi", "<" };
 
     /** The regular expression used to match serialized types. */
     private final Pattern allowlistPattern;
@@ -53,9 +54,9 @@ public final class SafeObjectInputStream extends ObjectInputStream {
      */
     public SafeObjectInputStream(InputStream in) throws IOException {
         super(in);
-        String safeObjectsProp = getPropertyValue("SafeObjectInputStream", "ListOfSafeObjectsForInputStream", "");
-        String[] allowlist = safeObjectsProp.isEmpty() ? DEFAULT_ALLOWLIST_PATTERN : safeObjectsProp.split(",");
-        allowlistPattern = Arrays.stream(allowlist)
+        String allowListProp = getPropertyValue("SafeObjectInputStream", "allowList", "");
+        String[] allowList = allowListProp.isEmpty() ? DEFAULT_ALLOWLIST_PATTERN : allowListProp.split(",");
+        allowlistPattern = Arrays.stream(allowList)
                 .map(String::trim)
                 .filter(str -> !str.isEmpty())
                 .collect(collectingAndThen(joining("|", "(", ")"), Pattern::compile));
@@ -65,9 +66,13 @@ public SafeObjectInputStream(InputStream in) throws IOException {
     protected Class<?> resolveClass(ObjectStreamClass classDesc) throws IOException, ClassNotFoundException {
         String className = classDesc.getName();
         // DenyList
-        if (className.contains("java.rmi") // Don't allow RMI
-                || className.contains("<")) { // Prevent generics markup in string type names
-            throw new InvalidClassException(className, "Unauthorized deserialisation attempt");
+        String rejectedObjectsProp = getPropertyValue("security", "denyList", "");
+        String[] denyList = rejectedObjectsProp.isEmpty() ? DEFAULT_DENYLIST : rejectedObjectsProp.split(",");
+        // For now DEFAULT_DENYLIST: don't allow RMI, prevent generics markup in string type names
+        for (String deny : denyList) {
+            if (className.contains(deny)) {
+                throw new InvalidClassException(className, "Unauthorized deserialisation attempt");
+            }
         }
         if (!allowlistPattern.matcher(className).find()) {
             Debug.logWarning("***Incompatible class***: " + className
