Skip to content

Conversation

@tvaron3
Copy link
Member

@tvaron3 tvaron3 commented Dec 22, 2025

Description

Whenever deserializing input the deserialization should only happen on specific allowed classes to prevent malicious input into deserializing unwanted classes. To exploit this vulnerability, the attacker needs access to the task configs of the kafka connector to edit this field with a malicious payload "azure.cosmos.client.metadata.caches.snapshot". For more information on deserialization vulnerabilities read here

@tvaron3 tvaron3 requested review from a team and kirankumarkolli as code owners December 22, 2025 19:26
Copilot AI review requested due to automatic review settings December 22, 2025 19:26
@tvaron3 tvaron3 requested a review from a team as a code owner December 22, 2025 19:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses an unsafe deserialization vulnerability in the Kafka connector by implementing a whitelist-based approach to prevent malicious classes from being deserialized. When deserializing client metadata cache snapshots, the code now checks that only allowed classes can be deserialized by overriding resolveClass in ObjectInputStream. A new test verifies that malicious payloads are blocked during deserialization.

Key Changes:

  • Added whitelist validation in the deserialization path to prevent RCE attacks
  • Created test case demonstrating that malicious deserialization attempts are blocked
  • Removed unused test configuration entries

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
sdk/cosmos/azure-cosmos-kafka-connect/src/main/java/com/azure/cosmos/kafka/connect/implementation/KafkaCosmosUtils.java Implements whitelist-based deserialization protection by overriding resolveClass method
sdk/cosmos/azure-cosmos-kafka-connect/src/test/java/com/azure/cosmos/kafka/connect/CosmosSinkConnectorTest.java Adds Evil test class and test case to verify malicious deserialization is blocked; removes unused patch operation config entries

Critical Issue Identified: The whitelist implementation has a critical flaw - it uses substring matching instead of exact class name matching, and the whitelist is incomplete, missing several nested classes that are legitimately required for deserialization. This will cause all deserializations to fail, including valid ones.

@tvaron3
Copy link
Member Author

tvaron3 commented Dec 22, 2025

/azp run java - cosmos - kafka

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tvaron3
Copy link
Member Author

tvaron3 commented Dec 22, 2025

/azp run java - cosmos - kafka

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@kushagraThapar kushagraThapar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @tvaron3

protected Class<?> resolveClass(ObjectStreamClass desc) throws IOException, ClassNotFoundException {
// Whitelist only allowed classes to prevent rce from arbitrary classes
if (!ALLOWED_CLASSES.contains(desc.getName())) {
LOGGER.error(desc.getName());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we add some message here, like "malicious attempt to deserialize" or maybe less scary message like "invalid class type for deserialization", or something like that?

Copy link
Member

@FabianMeiswinkel FabianMeiswinkel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - Thx

Copy link
Member

@xinlian12 xinlian12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the quick fix :)

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants