Restricting classes accepted during Zookeeper distributed queue deserialization (CWE-502) by Colhodm · Pull Request #110 · COG-GTM/BroadleafCommerce

Colhodm · 2026-08-27T22:44:51Z

A Brief Overview

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

The build targets Java 17, so the fix uses the JDK's own ObjectInputFilter rather than a custom resolveClass subclass:

ois = new ObjectInputStream(bais);
ois.setObjectInputFilter(getDeserializationFilter());   // allowlist + resource limits
return ois.readObject();

The filter is an allowlist terminated with !*, so anything unlisted is rejected: JDK value types (boxed primitives, String, Number, Enum, BigDecimal/BigInteger, Date, UUID, java.time.*), the common collections, java.lang.Object and java.util.Map$Entry (needed for the Object[] / Map.Entry[] backing arrays JDK serialization emits for those collections), and org.broadleafcommerce.** — queue elements are T extends Serializable supplied by application code, and the maxCapacity config node is an Integer. Limits maxdepth=16;maxrefs=10000;maxbytes=1000000;maxarray=10000 bound the resource cost of a hostile payload, since allowing Object also allows Object[].

Deployments that queue element types outside org.broadleafcommerce extend the allowlist without subclassing:

queue.setAdditionalAllowedClassPatterns("com.[REDACTED SECRET].MyQueueEntry");

Extra patterns are inserted before the trailing !*, and the assembled filter is cached in a volatile field (invalidated by the setter) because deserialize runs on every poll. A rejected class surfaces as a DistributedQueueException whose message names the allowlist and the setter, instead of the generic "unable to deserialize" wrapper.

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

Additional context

serialize() and all queue/locking behavior are unchanged. New test ZookeeperDistributedQueueDeserializationFilterTest round-trips Integer/String/ArrayList/HashMap through the filter, asserts java.io.File is rejected with InvalidClassException, and asserts buildDeserializationFilter("java.io.File") allows it — proving additional patterns land before the terminator. Verified with mvn -pl core/broadleaf-framework -am -DskipTests install and mvn -pl core/broadleaf-framework -Dtest=ZookeeperDistributedQueueDeserializationFilterTest test (3 tests, 0 failures).

Devin-Org: engineering

Link to Devin session: https://app.devin.ai/sessions/b0d17dc9564143c09e3a9812d82abdf3
Open in Devin Desktop: https://app.devin.ai/desktop/session/b0d17dc9564143c09e3a9812d82abdf3?variant=devin
Requested by: @Colhodm

Devin Review

Status	Commit
🟢 Reviewed	`1c94d3f`

Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>

devin-ai-integration · 2026-08-27T22:44:55Z

🤖 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-27T22:46:47Z

+    /**
+     * Allowlisted class patterns; the {@code ;!*} terminator is appended by {@link #buildDeserializationFilter(String)}.
+     */
+    protected static final String DEFAULT_ALLOWED_CLASS_PATTERNS = "maxdepth=16;maxrefs=10000;maxbytes=1000000;maxarray=10000;java.lang.Boolean;java.lang.Byte;java.lang.Character;java.lang.Short;java.lang.Integer;java.lang.Long;java.lang.Float;java.lang.Double;java.lang.String;java.lang.Number;java.lang.Enum;java.math.BigDecimal;java.math.BigInteger;java.util.Date;java.util.UUID;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.Optional;java.time.*;org.broadleafcommerce.**;java.lang.Object;java.util.Map$Entry";


🔴 Distributed incremental indexing rejects documents

In distributed Solr mode, DEFAULT_ALLOWED_CLASS_PATTERNS excludes SolrInputDocument, although every incremental command contains these documents. Polling fails before the index update executes.

Prompt for agents

The built-in distributed Solr queue stores IncrementalUpdateCommand instances containing org.apache.solr.common.SolrInputDocument objects. The default filter in core/broadleaf-framework/src/main/java/org/broadleafcommerce/core/util/queue/ZookeeperDistributedQueue.java permits Broadleaf command classes and standard collections but rejects the Solr document classes in their serialization graph. Audit a representative IncrementalUpdateCommand serialization graph and add the narrowly required Solr classes to the default allowlist. Add an integration-style filter test that round-trips an IncrementalUpdateCommand containing a populated SolrInputDocument, not only standard collection examples.

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

Debug

Playground

staging-devin-ai-integration · 2026-08-27T22:46:48Z

+    public void setAdditionalAllowedClassPatterns(String additionalAllowedClassPatterns) {
+        this.additionalAllowedClassPatterns = additionalAllowedClassPatterns;
+        this.deserializationFilter = null;


🟡 Concurrent allowlist updates stay stale

If setAdditionalAllowedClassPatterns overlaps filter construction, getDeserializationFilter can publish the old filter after invalidation. Later queue reads ignore the new patterns indefinitely.

Prompt for agents

The mutable additionalAllowedClassPatterns value and cached deserializationFilter are updated without one shared synchronization strategy. A thread in getDeserializationFilter can capture old patterns, race with setAdditionalAllowedClassPatterns invalidating the cache, and then repopulate that cache with the old filter. Make pattern updates and lazy filter publication atomic with respect to each other, using synchronization or an immutable holder containing both configuration and filter. Add a deterministic concurrency test or refactor so stale publication is structurally impossible.

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

Debug

Playground

staging-devin-ai-integration · 2026-08-27T22:46:49Z

+    @Test
+    public void shouldAllowAllowlistedClasses() throws Exception {
+        assertEquals(Integer.valueOf(42), deserialize(serialize(42), ZookeeperDistributedQueue.buildDeserializationFilter(null)));
+        assertEquals("value", deserialize(serialize("value"), ZookeeperDistributedQueue.buildDeserializationFilter(null)));
+
+        List<String> list = new ArrayList<>();
+        list.add("value");
+        assertEquals(list, deserialize(serialize(list), ZookeeperDistributedQueue.buildDeserializationFilter(null)));
+
+        HashMap<String, Integer> map = new HashMap<>();
+        map.put("value", 42);
+        assertEquals(map, deserialize(serialize(map), ZookeeperDistributedQueue.buildDeserializationFilter(null)));
+    }
+
+    @Test(expected = InvalidClassException.class)
+    public void shouldRejectNonAllowlistedClasses() throws Exception {
+        deserialize(serialize(new File("value")), ZookeeperDistributedQueue.buildDeserializationFilter(null));
+    }
+
+    @Test
+    public void shouldAllowAdditionalClassPatterns() throws Exception {
+        Object result = deserialize(
+                serialize(new File("value")),
+                ZookeeperDistributedQueue.buildDeserializationFilter("java.io.File")
+        );
+
+        assertEquals(new File("value"), result);
+    }


🔍 Tests omit the production payload

The tests cover standard collections but not the built-in IncrementalUpdateCommand graph. Add a populated production command to catch nested-class allowlist regressions.

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

Debug

Playground

staging-devin-ai-integration · 2026-08-27T22:46:50Z

+        if (additionalPatterns != null && !additionalPatterns.trim().isEmpty()) {
+            effectivePatterns += ";" + additionalPatterns;
+        }
+        effectivePatterns += ";!*";


🟥 Allowlist extensions bypass class restrictions

A caller can pass * through setAdditionalAllowedClassPatterns. The resulting filter accepts every class before reaching the final rejection rule.

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

Debug

Playground

devin-ai-integration Bot and others added 2 commits August 27, 2026 22:42

bug: filter classes during Zookeeper queue deserialization (CWE-502)

674687f

Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>

bug: tighten Zookeeper queue deserialization filter

1c94d3f

Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>

devin-ai-integration Bot assigned Colhodm Aug 27, 2026

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

View reviewed changes

Navigation Menu

Uh oh!

Restricting classes accepted during Zookeeper distributed queue deserialization (CWE-502) - #110

Restricting classes accepted during Zookeeper distributed queue deserialization (CWE-502)#110
Colhodm wants to merge 2 commits into
develop-7.0.xfrom
devin/1787870071-zk-queue-deserialization-filter

Colhodm commented Aug 27, 2026 •

edited by staging-devin-ai-integration Bot

Loading

Uh oh!

devin-ai-integration Bot commented Aug 27, 2026

Uh oh!

staging-devin-ai-integration Bot left a comment

Uh oh!

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

Uh oh!

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

Uh oh!

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

Uh oh!

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

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Colhodm commented Aug 27, 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 27, 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 27, 2026

Choose a reason for hiding this comment

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Colhodm commented Aug 27, 2026 •

edited by staging-devin-ai-integration Bot

Loading