-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fix Unsafe Deserialization #47594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix Unsafe Deserialization #47594
Conversation
There was a problem hiding this 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.
...ka-connect/src/main/java/com/azure/cosmos/kafka/connect/implementation/KafkaCosmosUtils.java
Outdated
Show resolved
Hide resolved
|
/azp run java - cosmos - kafka |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run java - cosmos - kafka |
|
Azure Pipelines successfully started running 1 pipeline(s). |
kushagraThapar
left a comment
There was a problem hiding this 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()); |
There was a problem hiding this comment.
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?
FabianMeiswinkel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - Thx
There was a problem hiding this 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 :)
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