Restricting ZookeeperDistributedQueue deserialization to an allow list of classes by Colhodm · Pull Request #100 · COG-GTM/BroadleafCommerce

Colhodm · 2026-08-17T22:39:43Z

A Brief Overview

ZookeeperDistributedQueue.deserialize() called ObjectInputStream.readObject() on bytes read from Zookeeper with no class filtering, so anyone able to write to the queue's znodes could drive gadget-chain RCE (CWE-502).

Every ObjectInputStream created in deserialize() now gets a JDK ObjectInputFilter built from a deny-by-default pattern:

maxdepth=32;maxrefs=2000;maxbytes=1048576;maxarray=10000;
<boxed primitives, String, Object, BigDecimal/BigInteger, java.time.*, Date, UUID,
 common java.util collections + Map$Entry, org.broadleafcommerce.**>;
!*

Rejections surface as a DistributedQueueException naming the system property to extend the list, rather than a bare IOException.

Extension points:

broadleaf.zookeeper.queue.deserialization.allowedClasses — semicolon-delimited patterns (e.g. com.mycompany.**) appended to the allow list, for applications queuing their own entry types.
createDeserializationFilter() — overridable for full control; the filter is built lazily and cached.

Note: org.broadleafcommerce.** is allowed, so queue payloads made of framework types keep working unchanged; application-specific payload classes outside that package need the system property.

Labels: Security, critical, ready-for-code-review

Additional context

Added ZookeeperDistributedQueueDeserializationTest, covering allowed collections round-tripping, a disallowed type (java.io.File) rejected both standalone and nested inside an allowed collection, and the system property widening the list. mvn -pl core/broadleaf-framework test -Dtest=ZookeeperDistributedQueueDeserializationTest passes (4 tests).

Link to Devin session: https://app.devin.ai/sessions/1fb3425711f647e2961c7e4712570d24
Requested by: @Colhodm

Devin Review

Status	Commit
🟢 Reviewed	`2796acb`

…tInputFilter allow list (CWE-502) Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>

devin-ai-integration · 2026-08-17T22:39:46Z

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

Disable automatic comment, CI, and merge conflict monitoring

staging-devin-ai-integration

Devin Review found 4 potential issues.

Debug

Playground

staging-devin-ai-integration · 2026-08-17T22:41:56Z

+    protected static final String DEFAULT_ALLOWED_CLASSES =
+            "java.lang.Boolean;java.lang.Byte;java.lang.Character;java.lang.Double;java.lang.Enum;java.lang.Float;"
+                    + "java.lang.Integer;java.lang.Long;java.lang.Number;java.lang.Object;java.lang.Short;java.lang.String;"
+                    + "java.math.BigDecimal;java.math.BigInteger;java.time.*;java.util.Date;"
+                    + "java.util.ArrayList;java.util.LinkedList;java.util.HashMap;java.util.LinkedHashMap;java.util.TreeMap;"
+                    + "java.util.HashSet;java.util.LinkedHashSet;java.util.TreeSet;java.util.UUID;java.util.Map$Entry;"
+                    + "org.broadleafcommerce.**";


🔴 Distributed Solr index updates stop working because the queue rejects its own messages

The list of types the queue is permitted to read back (DEFAULT_ALLOWED_CLASSES at core/broadleaf-framework/src/main/java/org/broadleafcommerce/core/util/queue/ZookeeperDistributedQueue.java:92-98) leaves out the Solr document types that the framework itself places on this queue, so every attempt to read a queued index-update message fails with an error.
Impact: In clustered deployments, incremental Solr index updates can no longer be queued or processed, so search index changes stop being applied.

Allow list omits org.apache.solr.common types carried by IncrementalUpdateCommand

The only production user of this queue is DefaultSolrIndexQueueProvider.createDistributedQueue (core/broadleaf-framework/src/main/java/org/broadleafcommerce/core/search/service/solr/indexer/DefaultSolrIndexQueueProvider.java:148), which stores SolrUpdateCommand instances. IncrementalUpdateCommand holds a List<SolrInputDocument> (core/broadleaf-framework/src/main/java/org/broadleafcommerce/core/search/service/solr/indexer/IncrementalUpdateCommand.java:30), i.e. org.apache.solr.common.SolrInputDocument / SolrInputField, which are not matched by any pattern in the allow list and are therefore rejected by the trailing !*.

Because AbstractSolrIndexUpdateServiceImpl.scheduleCommand calls commandQueue.contains(...) (core/broadleaf-framework/src/main/java/org/broadleafcommerce/core/search/service/solr/indexer/AbstractSolrIndexUpdateServiceImpl.java:156), which reads and deserializes existing entries, both writing and consuming break once any incremental command is on the queue: deserialize converts the filter's InvalidClassException into a DistributedQueueException.

Also note that immutable/wrapper collection implementations commonly produced by List.of(...), Arrays.asList(...) and Collections.unmodifiable* (java.util.ImmutableCollections$*, java.util.Arrays$ArrayList, java.util.Collections$*) are likewise absent from the allow list.

Suggested change

protected static final String DEFAULT_ALLOWED_CLASSES =

"java.lang.Boolean;java.lang.Byte;java.lang.Character;java.lang.Double;java.lang.Enum;java.lang.Float;"

+ "java.lang.Integer;java.lang.Long;java.lang.Number;java.lang.Object;java.lang.Short;java.lang.String;"

+ "java.math.BigDecimal;java.math.BigInteger;java.time.*;java.util.Date;"

+ "java.util.ArrayList;java.util.LinkedList;java.util.HashMap;java.util.LinkedHashMap;java.util.TreeMap;"

+ "java.util.HashSet;java.util.LinkedHashSet;java.util.TreeSet;java.util.UUID;java.util.Map$Entry;"

+ "org.broadleafcommerce.**";

protected static final String DEFAULT_ALLOWED_CLASSES =

"java.lang.Boolean;java.lang.Byte;java.lang.Character;java.lang.Double;java.lang.Enum;java.lang.Float;"

+ "java.lang.Integer;java.lang.Long;java.lang.Number;java.lang.Object;java.lang.Short;java.lang.String;"

+ "java.math.BigDecimal;java.math.BigInteger;java.time.*;java.util.Date;"

+ "java.util.ArrayList;java.util.LinkedList;java.util.HashMap;java.util.LinkedHashMap;java.util.TreeMap;"

+ "java.util.HashSet;java.util.LinkedHashSet;java.util.TreeSet;java.util.UUID;java.util.Map$Entry;"

+ "java.util.Arrays$ArrayList;java.util.Collections$*;java.util.ImmutableCollections$*;"

+ "org.apache.solr.common.**;org.broadleafcommerce.**";

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

staging-devin-ai-integration · 2026-08-17T22:41:57Z

+    /**
+     * Resource consumption limits applied on top of the class allow list to guard against deserialization bombs.
+     */
+    protected static final String DESERIALIZATION_LIMITS = "maxdepth=32;maxrefs=2000;maxbytes=1048576;maxarray=10000";


🟡 Larger queued messages are rejected by overly tight size limits

The resource limits applied when reading queue messages (DESERIALIZATION_LIMITS at core/broadleaf-framework/src/main/java/org/broadleafcommerce/core/util/queue/ZookeeperDistributedQueue.java:103) cap the number of objects at 2000 and array sizes at 10000, which realistic batched index-update messages exceed, so those messages are refused.
Impact: Larger batches of queued work fail to be read back and the operation errors out instead of completing.

maxrefs/maxbytes vs. the queue's documented 1MB payload allowance

maxrefs=2000 counts every back-referenceable object in the stream. An IncrementalUpdateCommand carrying a list of Solr documents (each document holding a map of SolrInputField objects and their string values) will easily surpass 2000 objects well before the 1MB Zookeeper transport limit that serialize warns about (core/broadleaf-framework/src/main/java/org/broadleafcommerce/core/util/queue/ZookeeperDistributedQueue.java:872). Similarly maxbytes=1048576 is exactly the Zookeeper limit, so a payload at that size is on the boundary of rejection. Consider raising maxrefs/maxarray, or making the limits configurable alongside ADDITIONAL_ALLOWED_CLASSES_PROPERTY.

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

staging-devin-ai-integration · 2026-08-17T22:41:58Z

        ObjectInputStream ois = null;
        try {
            ois = new ObjectInputStream(bais);
+            ois.setObjectInputFilter(getDeserializationFilter());


🔍 setObjectInputFilter can throw IllegalStateException when a JVM filter factory is configured

ObjectInputStream.setObjectInputFilter throws IllegalStateException if the stream's current filter is non-null and is not the process-wide filter — which happens when a deployment configures jdk.serialFilterFactory that assigns a per-stream filter. That unchecked exception is not caught here and would propagate out of deserialize as an IllegalStateException rather than a DistributedQueueException. Worth considering whether to combine with (rather than replace) any pre-existing stream filter.

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

staging-devin-ai-integration · 2026-08-17T22:42:00Z

+    protected static String buildDeserializationFilterPattern() {
+        StringBuilder pattern = new StringBuilder(DESERIALIZATION_LIMITS).append(';').append(DEFAULT_ALLOWED_CLASSES);
+        String additionalAllowedClasses = System.getProperty(ADDITIONAL_ALLOWED_CLASSES_PROPERTY);
+        if (additionalAllowedClasses != null && !additionalAllowedClasses.trim().isEmpty()) {
+            pattern.append(';').append(additionalAllowedClasses.trim());
+        }
+
+        //Reject anything that was not explicitly allowed above.
+        return pattern.append(";!*").toString();
+    }


🟨 Allow list permits any org.broadleafcommerce class plus arbitrary user-supplied patterns

The deserialization allow list ends with the broad wildcard org.broadleafcommerce.** and additionally appends whatever semicolon-delimited patterns are found in the broadleaf.zookeeper.queue.deserialization.allowedClasses system property. Any Serializable class in the framework package space (including classes with side-effecting readObject/readResolve implementations) remains reachable from untrusted Zookeeper data, and a misconfigured property (e.g. * or java.**) silently re-opens the unrestricted deserialization the filter is meant to prevent, since no validation is done on the supplied patterns.

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

bug: restrict ZookeeperDistributedQueue deserialization with an Objec…

2796acb

…tInputFilter allow list (CWE-502) Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>

Colhodm self-assigned this Aug 17, 2026

staging-devin-ai-integration Bot reviewed Aug 17, 2026

View reviewed changes

Navigation Menu

Uh oh!

Restricting ZookeeperDistributedQueue deserialization to an allow list of classes - #100

Restricting ZookeeperDistributedQueue deserialization to an allow list of classes#100
Colhodm wants to merge 1 commit into
develop-7.0.xfrom
devin/1787006370-zk-queue-deserialization-filter

Colhodm commented Aug 17, 2026 •

edited by staging-devin-ai-integration Bot

Loading

Uh oh!

devin-ai-integration Bot commented Aug 17, 2026

Uh oh!

staging-devin-ai-integration Bot left a comment

Uh oh!

staging-devin-ai-integration Bot Aug 17, 2026

Uh oh!

staging-devin-ai-integration Bot Aug 17, 2026

Uh oh!

staging-devin-ai-integration Bot Aug 17, 2026

Uh oh!

staging-devin-ai-integration Bot Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Colhodm commented Aug 17, 2026 • edited by staging-devin-ai-integration Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Devin Review

Uh oh!

devin-ai-integration Bot commented Aug 17, 2026

🤖 Devin AI Engineer

Uh oh!

staging-devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

staging-devin-ai-integration Bot Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

staging-devin-ai-integration Bot Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

staging-devin-ai-integration Bot Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

staging-devin-ai-integration Bot Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Colhodm commented Aug 17, 2026 •

edited by staging-devin-ai-integration Bot

Loading