Core: Restrict GenericAvroReader class resolution to an explicit allowlist - #17852
Open
damansingh1313 wants to merge 1 commit into
Open
Core: Restrict GenericAvroReader class resolution to an explicit allowlist#17852damansingh1313 wants to merge 1 commit into
damansingh1313 wants to merge 1 commit into
Conversation
…wlist GenericAvroReader.recordReader used an Avro schema's record name directly as a Java class name, loading and constructing it via reflection with no restriction. Two production paths feed this untrusted bytes/schemas: ManifestFiles.decode and Kafka Connect's AvroUtil.decode. If a schema named a class that happened to already be on the classpath and implement IndexedRecord, that class would be constructed regardless of whether it was ever intended to be reconstructed this way. This adds a hardcoded allowlist (ALLOWED_RECORD_CLASSES) inside GenericAvroReader and only attempts class resolution for names on that list; anything else now falls back to the existing generic record reader. Kafka Connect's classes are listed by name rather than Class literal since iceberg-core cannot depend on iceberg-kafka-connect-events; a comment cross-references that module's FIELD_ID_TO_CLASS to keep the two from drifting apart. This keeps the change self-contained to GenericAvroReader, with no changes to AvroEncoderUtil, ManifestFiles, or AvroUtil's method signatures or call sites. AI assistance disclosure: Claude Code was used to research the vulnerable code paths, draft this fix and its tests, and verify the build/test suite locally, per the project's AI-assisted contribution guidelines. The design (hardcoded allowlist inside the reader, no new parameters) was proposed after discussion with a maintainer on the issue; the approach and all code were reviewed and understood before submission. Closes apache#17802
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GenericAvroReader.recordReader used an Avro schema's record name directly as a Java class name, loading and constructing it via reflection with no restriction. Two production paths feed this untrusted bytes/schemas: ManifestFiles.decode and Kafka Connect's AvroUtil.decode. If a schema named a class that happened to already be on the classpath and implement IndexedRecord, that class would be constructed regardless of whether it was ever intended to be reconstructed this way.
This adds a hardcoded allowlist (ALLOWED_RECORD_CLASSES) inside GenericAvroReader and only attempts class resolution for names on that list; anything else now falls back to the existing generic record reader. Kafka Connect's classes are listed by name rather than Class literal since iceberg-core cannot depend on iceberg-kafka-connect-events; a comment cross-references that module's FIELD_ID_TO_CLASS to keep the two from drifting apart.
This keeps the change self-contained to GenericAvroReader, with no changes to AvroEncoderUtil, ManifestFiles, or AvroUtil's method signatures or call sites.
Closes #17802