Skip to content

Restricting ZookeeperDistributedQueue deserialization to an allowlist (CWE-502) - #115

Open
Colhodm wants to merge 1 commit into
develop-7.0.xfrom
devin/1788993473-zk-queue-deserialization-filter
Open

Colhodm wants to merge 1 commit into
develop-7.0.xfrom
devin/1788993473-zk-queue-deserialization-filter

Conversation

@Colhodm

@Colhodm Colhodm commented Sep 9, 2026

Copy link
Copy Markdown

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 znodes could trigger arbitrary gadget-chain deserialization (CWE-502 / RCE).

deserialize() now installs a JDK ObjectInputFilter on the stream before readObject():

protected ObjectInputFilter getDeserializationFilter() {
    return ObjectInputFilter.Config.createFilter(DEFAULT_DESERIALIZATION_FILTER_PATTERN);
}
// DEFAULT_DESERIALIZATION_FILTER_PATTERN =
//   maxdepth=64;maxarray=100000;maxrefs=100000;maxbytes=1048576;
//   java.lang.*;java.util.*;java.math.*;java.time.*;java.sql.Timestamp;java.sql.Date;
//   org.broadleafcommerce.**;org.apache.solr.common.**;!*
  • Allowlist covers what the framework actually puts on the queue: the Integer max-capacity config node, SolrUpdateCommand subclasses (IncrementalUpdateCommand carries List<SolrInputDocument>, hence org.apache.solr.common.**) and JDK value/collection types. Everything else is !* rejected.
  • maxbytes matches Zookeeper's 1MB transport limit; depth/array/ref caps guard against resource-exhaustion payloads.
  • A rejected class surfaces as DistributedQueueException (wrapping InvalidClassException) rather than being silently instantiated.
  • getDeserializationFilter() is protected so deployments queuing other types can widen the allowlist in a subclass without reintroducing unfiltered deserialization.

Additional context
New unit test ZookeeperDistributedQueueDeserializationTest round-trips the allowed types (Integer, IncrementalUpdateCommand with a SolrInputDocument) and asserts a non-allowlisted class is rejected both at top level and when nested inside an allowed ArrayList. Ran with mvn -pl core/broadleaf-framework -Dtest=ZookeeperDistributedQueueDeserializationTest test on JDK 17.

Labels: Security, Bug / critical.

Devin-Org: engineering

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

…alization (CWE-502)

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

Copy link
Copy Markdown

🤖 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