Security: fix incomplete Javadoc injection escaping in Builder accessor methods - #606
Merged
SrilakshmiBharadwaj merged 2 commits intoAug 26, 2026
Conversation
…or methods PR linkedin#604 (commit 3ed1779) introduced sanitizeDocForJavadoc() to escape '$' and '*/' in doc strings before embedding them in generated Javadoc, and applied it to class/enum/fixed-level doc and field declaration doc. However getFieldJavaDoc() - used by the generated Builder inner class's get/set/has/clear accessor method Javadoc - still called the older replaceSingleDollarSignWithDouble(), which does not escape '*/'. A schema with a hostile doc string containing '*/' can therefore still break out of the generated Builder accessor Javadoc comment blocks, reproducing the original CVE-2024-47561-style injection in 4 additional locations per field. Verified against a hostile-doc .avsc fixture: pre-fix, javac fails to compile the generated class (broken comment blocks); post-fix, the Builder accessor Javadoc is properly escaped and the generated class compiles cleanly.
… value-generation helpers validatedSpecificClassRef() (added in linkedin#604) is correctly used when declaring a field's *type* (schemaToJavaType() for RECORD/ENUM/FIXED), but three sibling methods that generate the actual instantiation/ invocation code independently re-derived the class name via codeModel.ref(AvroCompatibilityHelper.getSchemaFullName(...)), bypassing the validation entirely: - getEnumValueByName(): generated '<SchemaClass>.valueOf(name)' - getEnumValueByIndex(): generated 'Enums.getConstant(<SchemaClass>.class, index)' - getFixedValue(): generated 'new <SchemaClass>(bytes)' -- a direct, unvalidated constructor invocation getEnumValueByIndex/getEnumValueByName are on the live enum-field deserialization path in FastDeserializerGenerator. While today's call sites happen to validate the same schema earlier in the same generation pass via schemaToJavaType(), these three methods are public API on SchemaAssistant and had no independent guard, making the bypass trivially reachable by any future caller (including outside FastDeserializerGenerator) that doesn't validate first. Fixed by routing all three through validatedSpecificClassRef(), same as the type-declaration call sites. Verified with a standalone runtime harness: hostile enum/fixed schemas resolving to real classes that do NOT implement Enum/SpecificFixed are now blocked with SchemaAssistantException, while legitimate schemas (e.g. java.time.DayOfWeek as an Enum) are still allowed through unchanged.
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.
PR #604 (commit 3ed1779) introduced sanitizeDocForJavadoc() to escape '$' and '*/' in doc strings before embedding them in generated Javadoc, and applied it to class/enum/fixed-level doc and field declaration doc.
However getFieldJavaDoc() - used by the generated Builder inner class's get/set/has/clear accessor method Javadoc - still called the older replaceSingleDollarSignWithDouble(), which does not escape '/'. A schema with a hostile doc string containing '/' can therefore still break out of the generated Builder accessor Javadoc comment blocks, reproducing the original CVE-2024-47561-style injection in 4 additional locations per field.
Verified against a hostile-doc .avsc fixture: pre-fix, javac fails to compile the generated class (broken comment blocks); post-fix, the Builder accessor Javadoc is properly escaped and the generated class compiles cleanly.