From 0a2fcac7eadb9408d5961756c4dcfb2c88183a38 Mon Sep 17 00:00:00 2001 From: Jacopo Cappellato Date: Thu, 9 Apr 2026 09:11:34 +0200 Subject: [PATCH 1/4] Fixed: Remove support for serialization and deserialization of custom Java objects in XmlSerializer for security reasons --- .../ofbiz/entity/serialize/XmlSerializer.java | 34 +++++-------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/framework/entity/src/main/java/org/apache/ofbiz/entity/serialize/XmlSerializer.java b/framework/entity/src/main/java/org/apache/ofbiz/entity/serialize/XmlSerializer.java index 5581c76f268..b20bc313ff3 100644 --- a/framework/entity/src/main/java/org/apache/ofbiz/entity/serialize/XmlSerializer.java +++ b/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()); } From 9fcd6ad88b661670516ba6c2b5db44a07142572d Mon Sep 17 00:00:00 2001 From: Jacopo Cappellato Date: Thu, 9 Apr 2026 09:13:29 +0200 Subject: [PATCH 2/4] 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 --- .../base/util/SafeObjectInputStream.java | 31 +++++++++++++++++-- .../org/apache/ofbiz/base/util/UtilXml.java | 24 ++++++++------ .../ofbiz/base/util/UtilObjectTests.java | 5 +++ .../service/jms/AbstractJmsListener.java | 19 ++++++------ 4 files changed, 57 insertions(+), 22 deletions(-) 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 1bc1ba93f28..88eda3a8acf 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 @@ -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. */ diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilXml.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilXml.java index 414a1eacc50..085d1db45ec 100644 --- a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilXml.java +++ b/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; } diff --git a/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilObjectTests.java b/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilObjectTests.java index b6d7b2ef126..49e04930f6e 100644 --- a/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilObjectTests.java +++ b/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)); diff --git a/framework/service/src/main/java/org/apache/ofbiz/service/jms/AbstractJmsListener.java b/framework/service/src/main/java/org/apache/ofbiz/service/jms/AbstractJmsListener.java index 6bb91eb1d6d..9a897263067 100644 --- a/framework/service/src/main/java/org/apache/ofbiz/service/jms/AbstractJmsListener.java +++ b/framework/service/src/main/java/org/apache/ofbiz/service/jms/AbstractJmsListener.java @@ -71,6 +71,13 @@ protected Map 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 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); } From 314729b42d1725a07de4bced2b9218a03f83cdf5 Mon Sep 17 00:00:00 2001 From: Jacopo Cappellato Date: Wed, 15 Apr 2026 19:41:09 +0200 Subject: [PATCH 3/4] Fixed: Remove references to the no more used 'sun.util.calendar.*' package from (commented out) JVM argument and from serialization allow list --- build.gradle | 1 - framework/base/config/SafeObjectInputStream.properties | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 4963c7f0524..eb11a9578ec 100644 --- a/build.gradle +++ b/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 ] } diff --git a/framework/base/config/SafeObjectInputStream.properties b/framework/base/config/SafeObjectInputStream.properties index 9358acc1e6a..3e6fda2e746 100644 --- a/framework/base/config/SafeObjectInputStream.properties +++ b/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 From 6e75cde7576430eb6bf80db946c7dd91aa138f2c Mon Sep 17 00:00:00 2001 From: Jacopo Cappellato Date: Mon, 20 Apr 2026 10:19:17 +0200 Subject: [PATCH 4/4] Fixed: Extend allowlist in SafeObjectInputStream to include GString* Groovy classes The classes are also already included in the configurable allow list in SafeObjectInputStream.properties --- .../org/apache/ofbiz/base/util/SafeObjectInputStream.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 88eda3a8acf..aedea1a082e 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 @@ -66,7 +66,9 @@ public final class SafeObjectInputStream extends ObjectInputStream { "java\\.util\\.TreeMap", "java\\.util\\.Arrays\\$ArrayList", "org\\.apache\\.ofbiz\\.entity\\.GenericValue", - "org\\.apache\\.ofbiz\\.entity\\.GenericPK"}; + "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. */