Skip to content

Harden ZookeeperDistributedQueue deserialization against untrusted class instantiation (CWE-502)#52

Open
devin-ai-integration[bot] wants to merge 1 commit into
develop-7.0.xfrom
devin/1782945304-zk-queue-deserialization-hardening
Open

Harden ZookeeperDistributedQueue deserialization against untrusted class instantiation (CWE-502)#52
devin-ai-integration[bot] wants to merge 1 commit into
develop-7.0.xfrom
devin/1782945304-zk-queue-deserialization-hardening

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jul 1, 2026

Copy link
Copy Markdown

Purpose
Fixes an insecure deserialization vulnerability (CWE-502) in ZookeeperDistributedQueue.deserialize(). The method fed data read from Zookeeper straight into ObjectInputStream.readObject() with no class filtering, so anyone able to influence the bytes stored in Zookeeper could trigger a "gadget chain" and achieve remote code execution.

A Brief Overview
deserialize() now uses a hardened ObjectInputStream (ValidatingObjectInputStream) that validates every class against an allowlist before it is resolved, and rejects all dynamic proxy classes.

Behavior:

resolveClass(desc):
    if !isDeserializationClassAllowed(desc.getName()): throw InvalidClassException
    else super.resolveClass(desc)
resolveProxyClass(...): throw InvalidClassException   // proxies never allowed

The allowlist (getAllowedDeserializationClassNames(), seeded from DEFAULT_ALLOWED_DESERIALIZATION_CLASS_NAMES) accepts exact class names or package prefixes ending in .. Defaults cover the JDK value/collection types the queue itself relies on (e.g. the Integer capacity marker, java.util.*, java.time.*, java.math.*, common java.lang scalars) plus org.broadleafcommerce.*. Array encodings such as [Ljava.lang.String; and [[I are normalized to their base component type before matching; primitive-array components are always allowed.

Compatibility note: applications that place custom Serializable payloads on the queue must register those class names, e.g.:

queue.getAllowedDeserializationClassNames().add("com.example.MyQueueMessage");

Otherwise reads of those entries will fail with a wrapped InvalidClassException. This is the intended, fail-closed behavior for CWE-502.

Tests
Adds ZookeeperDistributedQueueDeserializationTest (6 tests, all passing) covering: scalar/collection/array round-trips for allowlisted types, rejection of a non-allowlisted Serializable (java.io.File) with an InvalidClassException cause, re-enabling it via the allowlist, and the class-name/array/gadget matching logic.

Run:

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

Labels

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

Additional context
Fix is confined to ZookeeperDistributedQueue; the serialize() path is unchanged, so entries written before this change remain readable as long as their types are on (or added to) the allowlist.

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


Devin Review

Status Commit
⚪ Not started

Run Devin Review

Open in Devin Review (Staging)

Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>
@Colhodm Colhodm self-assigned this Jul 1, 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