[ofbiz-framework] branch release18.12 updated: Improved: Improve ObjectInputStream denyList (OFBIZ-12221)

This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a commit to branch release18.12
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/release18.12 by this push:
     new 2c6f306  Improved: Improve ObjectInputStream denyList (OFBIZ-12221)
2c6f306 is described below

commit 2c6f3068de9d25f42c2fe7d762d1696ba3982dbb
Author: Jacques Le Roux <[hidden email]>
AuthorDate: Wed Apr 7 08:51:36 2021 +0200

    Improved: Improve ObjectInputStream denyList (OFBIZ-12221)
   
    In SafeObjectInputStream.properties
      Renames listOfSafeObjectsForInputStream to allowList and fixes it
      Introduces a denyList
   
    Adapts SafeObjectInputStream class to new denyList
   
    Conflicts handled by hand
      framework/base/config/SafeObjectInputStream.properties
      framework/base/src/main/java/org/apache/ofbiz/base/util/SafeObjectInputStream.java
---
 .../base/config/SafeObjectInputStream.properties   | 19 ++++++++-----
 .../ofbiz/base/util/SafeObjectInputStream.java     | 31 +++++++++++-----------
 2 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/framework/base/config/SafeObjectInputStream.properties b/framework/base/config/SafeObjectInputStream.properties
index 548eab7..9358acc 100644
--- 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 whitelist),
-# 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
index 5c1b8b6..185d828 100644
--- 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
@@ -37,26 +37,26 @@ import java.util.regex.Pattern;
  * only authorized class can be read from it.
  */
 public final class SafeObjectInputStream extends ObjectInputStream {
-    private static final String[] DEFAULT_WHITELIST_PATTERN = {
+    private static final String[] DEFAULT_ALLOWLIST_PATTERN = {
             "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"};
+    private static final String[] DEFAULT_DENYLIST = { "rmi", "<" };
 
     /** The regular expression used to match serialized types. */
-    private final Pattern whitelistPattern;
+    private final Pattern allowlistPattern;
 
     /**
      * Instantiates a safe object input stream.
-     *
      * @param in  the input stream to read
      * @throws IOException when reading is not possible.
      */
     public SafeObjectInputStream(InputStream in) throws IOException {
         super(in);
-        String safeObjectsProp = getPropertyValue("SafeObjectInputStream", "ListOfSafeObjectsForInputStream", "");
-        String[] whitelist = safeObjectsProp.isEmpty() ? DEFAULT_WHITELIST_PATTERN : safeObjectsProp.split(",");
-        whitelistPattern = Arrays.stream(whitelist)
+        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));
@@ -66,17 +66,16 @@ public final class SafeObjectInputStream extends ObjectInputStream {
     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");
-        }
-        if (!whitelistPattern.matcher(className).find()) {
-            // DiskFileItem, FileItemHeadersImpl are not serializable.
-            if (className.contains("org.apache.commons.fileupload")) {
-                throw new ClassNotFoundException("DiskFileItem and FileItemHeadersImpl are not serializable.");
+        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");
             }
-            Debug.logWarning("***Incompatible class***: "
-                    + classDesc.getName()
+        }
+        if (!allowlistPattern.matcher(className).find()) {
+            Debug.logWarning("***Incompatible class***: " + className
                     + ". Please see OFBIZ-10837.  Report to dev ML if you use OFBiz without changes. "
                     + "Else follow https://s.apache.org/45war",
                     "SafeObjectInputStream");