Mitigate insecure deserialization in ZookeeperDistributedQueue (CWE-502)

[devin-ai-integration]

Overview

ZookeeperDistributedQueue.deserialize(byte[]) previously called ObjectInputStream.readObject() on bytes read from Zookeeper with no class filtering, which is a CWE-502 insecure deserialization vulnerability. If an attacker can influence data in the Zookeeper znodes backing a queue, they can trigger RCE via known Java deserialization gadget chains.

This PR applies a default ObjectInputFilter to the ObjectInputStream used in deserialize() to block known gadget packages and enforce size/depth limits, and exposes a protected getObjectInputFilter() hook so subclasses that know their exact payload types can install a stricter allow-list based filter.

Changes

• ZookeeperDistributedQueue.java
  • Added DEFAULT_DESERIALIZATION_FILTER_PATTERN / DEFAULT_DESERIALIZATION_FILTER constants.
  • New protected ObjectInputFilter getObjectInputFilter() method returning the default filter. Subclasses can override.
  • deserialize() now calls ois.setObjectInputFilter(...) with whatever the hook returns (unless it returns null, which falls back to the JVM-wide jdk.serialFilter).
• ZookeeperDistributedQueueDeserializationFilterTest.java
  • Verifies common JDK payload types (String, Integer, Long, ArrayList, HashMap, Date) are still allowed.
  • Verifies real JDK/Spring classes from denied packages (JdbcRowSetImpl, TemplatesImpl, BadAttributeValueExpException, org.springframework.beans.factory.ObjectFactory) are rejected.
  • Verifies both org.springframework.core.SerializableTypeWrapper and its inner gadget class SerializableTypeWrapper$MethodInvokeTypeProvider are rejected.
  • Sanity-checks the pattern literally contains the intended denylist tokens.

Updates since last revision

• Fixed the SerializableTypeWrapper denylist entry. The original !org.springframework.core.SerializableTypeWrapper** pattern was ineffective — JDK ObjectInputFilter only treats .** (with a leading .) as a package wildcard, so a bare ** degenerated to an exact-string match and never fired. Replaced with !org.springframework.core.SerializableTypeWrapper* (trailing single * = class-name prefix match), which covers the outer class and all its inner gadget classes ($MethodInvokeTypeProvider, $TypeProxyInvocationHandler, etc.) without widening the denylist to the entire org.springframework.core package. Caught by Devin Review; verified with an added regression test on the inner class.

Labels

• Security
• Severity: critical
• Status: ready-for-code-review

Milestone

Upcoming 7.0.x release.

Things To Review Carefully

1. Denylist vs allow-list. Because T extends Serializable means callers can legitimately put arbitrary serializable payloads in the queue, the default has to be a denylist for backward compatibility. This is inherently weaker than an allow-list and will not catch future gadget chains. Consumers with a known, narrow payload type should override getObjectInputFilter() — please confirm this framework-level trade-off is acceptable, and consider whether any internal subclasses/callers should be switched to an allow-list filter in a follow-up.
2. Denylist coverage. The pattern blocks the well-known gadget sources from ysoserial / Marshalsec (Commons Collections functors, Groovy runtime, Spring beans.factory.** and SerializableTypeWrapper*, Xalan TemplatesImpl, JdbcRowSetImpl, c3p0, Hibernate engine.spi/property, RMI server, BeanShell, Clojure, Jython/org.python.core, ROME, Javassist, Rhino/org.mozilla.javascript, Vaadin, XBean naming, javax.naming.**). Please sanity-check this list against anything Broadleaf may intentionally put in a ZookeeperDistributedQueue — in particular org.springframework.beans.factory.** and javax.naming.** are broad and could in theory block legitimate payloads.
3. Resource limits. Defaults are maxdepth=64, maxarray=1_000_000, maxrefs=100_000, maxbytes=10_000_000. ZK itself limits entries to ~1MB so maxbytes=10MB is comfortably above that, but please confirm these bounds are reasonable for existing queue sizes.
4. Callers that override deserialize(). Any subclass that already overrides deserialize() won't pick up this mitigation automatically — they'd need to call getObjectInputFilter() themselves or upgrade their own implementation.
5. No Zookeeper-integration test. The new test exercises only the filter pattern (no real ZK round-trip), since the existing module does not appear to have a ZK-backed integration harness.

Additional Context

• CWE reference: CWE-502 Deserialization of Untrusted Data.
• Java reference: java.io.ObjectInputFilter (Java 9+; this module already targets Java 17 per root pom.xml).
• Operators can additionally set a process-wide filter via the jdk.serialFilter system property / conf/security/java.security for defense-in-depth.

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

---

[devin-ai-integration]

🤖 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 and CI monitoring