[ofbiz-framework] branch trunk 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 trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 3f97578  Improved: Improve ObjectInputStream denyList (OFBIZ-12221)
3f97578 is described below

commit 3f975780cfaba0231772ae1853f02a0de199928f
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
---
 .../base/config/SafeObjectInputStream.properties      | 19 ++++++++++++-------
 .../apache/ofbiz/base/util/SafeObjectInputStream.java | 17 +++++++++++------
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/framework/base/config/SafeObjectInputStream.properties b/framework/base/config/SafeObjectInputStream.properties
index ca6c829..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 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
index f9d93b2..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
@@ -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 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");
+        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