Skip to content

Harden ZookeeperDistributedQueue deserialization against insecure deserialization (CWE-502)#51

Open
devin-ai-integration[bot] wants to merge 1 commit into
develop-7.0.xfrom
devin/1782772424-zk-queue-deser-filter
Open

Harden ZookeeperDistributedQueue deserialization against insecure deserialization (CWE-502)#51
devin-ai-integration[bot] wants to merge 1 commit into
develop-7.0.xfrom
devin/1782772424-zk-queue-deser-filter

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jun 29, 2026

Copy link
Copy Markdown

A Brief Overview

ZookeeperDistributedQueue.deserialize() called ObjectInputStream.readObject() on bytes read back from Zookeeper with no class filtering. Any actor able to influence the data stored in the queue's Zookeeper nodes could supply a serialized "gadget chain" payload and achieve arbitrary class instantiation / remote code execution (CWE-502).

This PR applies a JDK ObjectInputFilter allowlist before any object in the stream is resolved, so only known-safe classes can be deserialized; everything else is rejected.

What changed

  • Added DEFAULT_DESERIALIZATION_ALLOWLIST, an ObjectInputFilter pattern that permits only standard JDK value/collection types (java.lang.*, java.util.* + concurrent, java.time.*, java.math.*, java.net.*, java.sql.*) and org.broadleafcommerce.**, ending in !* to reject everything else. It also caps stream size, graph depth, array length, and reference count to limit resource-exhaustion attacks.
  • deserialize(byte[]) now delegates to a new deserialize(byte[], ObjectInputFilter) that calls ois.setObjectInputFilter(filter) before readObject(). A rejected class surfaces as InvalidClassException, which is wrapped in the existing DistributedQueueException (no behavior change for callers).
  • Added overridable hooks getDeserializationAllowlist() / getDeserializationFilter() so subclasses that store custom serializable payloads can extend the allowlist while keeping the trailing !*.
ois = new ObjectInputStream(bais);
ois.setObjectInputFilter(getDeserializationFilter()); // allowlist; rejects unlisted classes
return ois.readObject();

Note: serialize() is unchanged — pre-existing queue entries written by trusted nodes use only allowlisted types and continue to round-trip.

Additional context

Added ZookeeperDistributedQueueDeserializationTest (5 tests, all passing) covering: allowlisted scalar/collection round-trips, rejection of a non-allowlisted Serializable class (java.io.File as a gadget stand-in) with an InvalidClassException cause, a control case proving the same payload deserializes when the filter is disabled, and that the allowlist terminates in !*.

Run with:

mvn -pl core/broadleaf-framework -am test -Dtest=ZookeeperDistributedQueueDeserializationTest -Dsurefire.failIfNoSpecifiedTests=false

Suggested labels: Bug, Security, Severity: critical, Status: ready-for-code-review

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


Devin Review

Status Commit
⚪ Not started

Run Devin Review

Open in Devin Review (Staging)

… deserialization (CWE-502)

Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>
@Colhodm Colhodm self-assigned this Jun 29, 2026
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant