Fixed: Prevent deserialization of untrusted data in JMS listener

- Restrict XStream allowlist in UtilXml to the exact types handled by
  XmlSerializer.serializeSingle
- Replace the broad java..* and org.apache.ofbiz..* wildcards in
  SafeObjectInputStream with an explicit minimal allowlist of the same
  safe types
- Move the isExport() authorization check before XmlSerializer.deserialize
  in AbstractJmsListener.runService so gadget chains cannot fire for
  non-exported or unknown services

Author: jacopoc

framework/base/src/main/java/org/apache/ofbiz/base/util/SafeObjectInputStream.java

@@ -38,10 +38,35 @@
  */
 public final class SafeObjectInputStream extends ObjectInputStream {
     private static final String[] DEFAULT_ALLOWLIST_PATTERN = {
-            "byte\\[\\]", "foo", "SerializationInjector",
+            "byte\\[\\]",
             "\\[Z", "\\[B", "\\[S", "\\[I", "\\[J", "\\[F", "\\[D", "\\[C",
-            "java..*", "sun.util.calendar..*", "org.apache.ofbiz..*",
-            "org.codehaus.groovy.runtime.GStringImpl", "groovy.lang.GString"};
+            "java\\.lang\\.String",
+            "java\\.lang\\.Number",
+            "java\\.lang\\.Integer",
+            "java\\.lang\\.Long",
+            "java\\.lang\\.Float",
+            "java\\.lang\\.Double",
+            "java\\.lang\\.Boolean",
+            "java\\.util\\.Locale",
+            "java\\.math\\.BigDecimal",
+            "java\\.sql\\.Timestamp",
+            "java\\.sql\\.Date",
+            "java\\.sql\\.Time",
+            "java\\.util\\.Date",
+            "java\\.util\\.ArrayList",
+            "java\\.util\\.LinkedList",
+            "java\\.util\\.Stack",
+            "java\\.util\\.Vector",
+            "java\\.util\\.TreeSet",
+            "java\\.util\\.HashSet",
+            "java\\.util\\.HashMap",
+            "java\\.util\\.Properties",
+            "java\\.util\\.Hashtable",
+            "java\\.util\\.WeakHashMap",
+            "java\\.util\\.TreeMap",
+            "java\\.util\\.Arrays\\$ArrayList",
+            "org\\.apache\\.ofbiz\\.entity\\.GenericValue",
+            "org\\.apache\\.ofbiz\\.entity\\.GenericPK"};
     private static final String[] DEFAULT_DENYLIST = {"rmi", "<"};
 
     /** The regular expression used to match serialized types. */

framework/base/src/main/java/org/apache/ofbiz/base/util/UtilXml.java

@@ -91,15 +91,21 @@ private UtilXml() { }
 
     private static XStream createXStream() {
         XStream xstream = new XStream();
-        /* This method is a pure helper method for XStream 1.4.x.
-         * It initializes an XStream instance with a white list of well-known and simple types of the Java runtime
-         *  as it is done in XStream 1.5.x by default. This method will do therefore nothing in XStream 1.5
-         *  and could be removed them
-         */
-        // XStream.setupDefaultSecurity(xstream);
-        /* You may want to enhance the white list created by XStream::setupDefaultSecurity (or by default with XStream 1.5)
-         * using xstream::allowTypesByWildcard with your own classes
-         */
+        // Allow only the concrete types that XmlSerializer.serializeSingle handles explicitly.
+        // All other types are blocked to prevent deserialization gadget chain attacks.
+        // Class names are used as strings to avoid a compile-time dependency on framework/entity.
+        xstream.allowTypes(new String[]{
+                "java.lang.String", "java.lang.Integer", "java.lang.Long",
+                "java.lang.Float", "java.lang.Double", "java.lang.Boolean",
+                "java.util.Locale", "java.math.BigDecimal",
+                "java.sql.Timestamp", "java.sql.Date", "java.sql.Time", "java.util.Date",
+                "java.util.ArrayList", "java.util.LinkedList", "java.util.Stack",
+                "java.util.Vector", "java.util.TreeSet", "java.util.HashSet",
+                "java.util.HashMap", "java.util.Properties", "java.util.Hashtable",
+                "java.util.WeakHashMap", "java.util.TreeMap",
+                "org.apache.ofbiz.entity.GenericValue",
+                "org.apache.ofbiz.entity.GenericPK"
+        });
         return xstream;
     }
 

framework/base/src/test/java/org/apache/ofbiz/base/util/UtilObjectTests.java

@@ -216,6 +216,11 @@ public void testGetObject() {
             assertNotNull("groovySerializableBytes", groovySerializableBytes);
             assertNull("groovyDeserializable", UtilObject.getObject(groovySerializableBytes));
 
+            // SerializationInjector is a test-only class; allow it explicitly for this assertion.
+            // Note: the allowList value is treated as a regex, so '.' is used instead of '$'
+            // (dot matches any character including the inner-class separator '$').
+            UtilProperties.setPropertyValueInMemory("SafeObjectInputStream", "allowList",
+                    "org.apache.ofbiz.base.util.UtilObjectTests.SerializationInjector");
             byte[] injectorBytes = UtilObject.getBytes(new SerializationInjector(false, false));
             assertNotNull("injectorBytes good", injectorBytes);
             assertNotNull("injector good", UtilObject.getObject(injectorBytes));

framework/service/src/main/java/org/apache/ofbiz/service/jms/AbstractJmsListener.java

@@ -71,6 +71,13 @@ protected Map<String, Object> runService(MapMessage message) {
                 return null;
             }
 
+            // Authorization check BEFORE deserialization to prevent gadget chain attacks.
+            ModelService model = dispatcher.getDispatchContext().getModelService(serviceName);
+            if (!model.isExport()) {
+                Debug.logWarning("Attempt to invoke a non-exported service: " + serviceName, MODULE);
+                return null;
+            }
+
             Object o = XmlSerializer.deserialize(xmlContext, dispatcher.getDelegator());
 
             if (Debug.verboseOn()) {
@@ -81,20 +88,12 @@ protected Map<String, Object> runService(MapMessage message) {
             }
         } catch (JMSException je) {
             Debug.logError(je, "Problems reading message.", MODULE);
+        } catch (GenericServiceException e) {
+            Debug.logError(e, "Unable to get ModelService for service: " + serviceName, MODULE);
         } catch (Exception e) {
             Debug.logError(e, "Problems deserializing the service context.", MODULE);
         }
 
-        try {
-            ModelService model = dispatcher.getDispatchContext().getModelService(serviceName);
-            if (!model.isExport()) {
-                Debug.logWarning("Attempt to invoke a non-exported service: " + serviceName, MODULE);
-                return null;
-            }
-        } catch (GenericServiceException e) {
-            Debug.logError(e, "Unable to get ModelService for service : " + serviceName, MODULE);
-        }
-
         if (Debug.verboseOn()) {
             Debug.logVerbose("Running service: " + serviceName, MODULE);
         }