Fixed: Prevent deserialization of untrusted data in JMS listener

build.gradle

@@ -121,7 +121,6 @@ application {
             ? jvmArgs.tokenize()
             : ['-Xms128M','-Xmx1024M',
             '-Djdk.serialFilter=maxarray=100000;maxdepth=20;maxrefs=1000;maxbytes=500000', // OFBIZ-12592 and OFBIZ-12716
-//            '--add-exports=java.base/sun.util.calendar=ALL-UNNAMED', // OFBIZ-12721
             '--add-opens=java.base/java.util=ALL-UNNAMED' // OFBIZ-12726
             ]
 }

framework/base/config/SafeObjectInputStream.properties

@@ -27,7 +27,7 @@
 #     . don't forget to add new objects in SafeObjectInputStream class too (as default there).
 #     . "foo" and "SerializationInjector" are used in OFBiz tests
 
-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
+allowList=byte\\[\\], foo, SerializationInjector, \\[Z,\\[B,\\[S,\\[I,\\[J,\\[F,\\[D,\\[C, java..*, 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

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

@@ -38,10 +38,37 @@
  */
 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",
+            "org\\.codehaus\\.groovy\\.runtime\\.GStringImpl",
+            "groovy\\.lang\\.GString"};
     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/entity/src/main/java/org/apache/ofbiz/entity/serialize/XmlSerializer.java

@@ -20,7 +20,6 @@
 
 import java.io.FileNotFoundException;
 import java.io.IOException;
-import java.io.Serializable;
 import java.lang.ref.WeakReference;
 import java.math.BigDecimal;
 import java.math.RoundingMode;
@@ -48,10 +47,8 @@
 import javax.xml.parsers.ParserConfigurationException;
 
 import org.apache.ofbiz.base.util.Debug;
-import org.apache.ofbiz.base.util.StringUtil;
 import org.apache.ofbiz.base.util.UtilGenerics;
 import org.apache.ofbiz.base.util.UtilMisc;
-import org.apache.ofbiz.base.util.UtilObject;
 import org.apache.ofbiz.base.util.UtilXml;
 import org.apache.ofbiz.entity.Delegator;
 import org.apache.ofbiz.entity.GenericPK;
@@ -259,18 +256,11 @@ public static Element serializeSingle(Object object, Document document) throws S
     }
 
     public static Element serializeCustom(Object object, Document document) throws SerializeException {
-        if (object instanceof Serializable) {
-            byte[] objBytes = UtilObject.getBytes(object);
-            if (objBytes == null) {
-                throw new SerializeException("Unable to serialize object; null byte array returned");
-            }
-            String byteHex = StringUtil.toHexString(objBytes);
-            Element element = document.createElement("cus-obj");
-            // this is hex encoded so does not need to be in a CDATA block
-            element.appendChild(document.createTextNode(byteHex));
-            return element;
-        }
-        throw new SerializeException("Cannot serialize object of class " + object.getClass().getName());
+        Debug.logError("Serialization of custom Java objects (cus-obj) is no longer supported. "
+                + "This feature has been removed for security reasons. Object class: "
+                + object.getClass().getName(), MODULE);
+        throw new SerializeException("Serialization of custom Java objects is not supported. "
+                + "Object class: " + object.getClass().getName());
     }
 
     public static Element makeElement(String elementName, Object value, Document document) {
@@ -467,17 +457,9 @@ public static Object deserializeSingle(Element element, Delegator delegator) thr
     public static Object deserializeCustom(Element element) throws SerializeException {
         String tagName = element.getLocalName();
         if ("cus-obj".equals(tagName)) {
-            String value = UtilXml.elementValue(element);
-            if (value != null) {
-                byte[] valueBytes = StringUtil.fromHexString(value);
-                if (valueBytes != null) {
-                    Object obj = UtilObject.getObject(valueBytes);
-                    if (obj != null) {
-                        return obj;
-                    }
-                }
-            }
-            throw new SerializeException("Problem deserializing object from byte array + " + element.getLocalName());
+            Debug.logError("Deserialization of cus-obj elements is no longer supported. "
+                    + "This feature has been removed for security reasons.", MODULE);
+            throw new SerializeException("Deserialization of cus-obj elements is not supported.");
         }
         throw new SerializeException("Cannot deserialize element named " + element.getLocalName());
     }

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);
         }