fix(serialization): enforce writeReplace security check for hessian2 - #16437
Open
GerardGao wants to merge 2 commits into
Open
fix(serialization): enforce writeReplace security check for hessian2#16437GerardGao wants to merge 2 commits into
GerardGao wants to merge 2 commits into
Conversation
hessian-lite 3.2.x resolves classes with a writeReplace() method to JavaSerializer directly and never reaches getDefaultSerializer(), so such classes skipped checkSerializable() entirely. Prepend a check factory that applies the same security check to writeReplace classes and falls through to the normal resolution chain. Fixes apache#16287
Author
|
Note on CI: the "Build and Test For PR" workflow exits with Local verification for this PR:
|
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
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.
What is the purpose of the change
hessian-lite 3.2.x resolves classes that declare a
writeReplace()method directly toJavaSerializer, beforegetDefaultSerializer()is ever consulted. Such classes therefore skipped thecheckSerializable()security check entirely: a class that does not implementSerializablecould be serialized as long as it declaredwriteReplace()(verified against 3.2 with a reproduction test). This is the serialization security bypass reported in #16287.The 3.3 line is unaffected (hessian-lite 4.0.5 checks the original class in the
writeReplacebranch).Brief changelog
Hessian2SerializerFactoryregisters a pre-flightAbstractSerializerFactoryviaaddFactorythat applies the existingcheckSerializable()to classes carrying awriteReplace()method, then returns null so the normalSerializerFactoryresolution chain keeps control.writeReplaceclasses in hessian-lite 4.x, scoped to the 3.2 line.Verifying this change
Hessian2WriteReplaceSecurityTest:Serializableclass withwriteReplace()is rejected;Serializableclass whosewriteReplace()returns a non-Serializableholder is rejected;Serializableclass whosewriteReplace()returns aStringstill serializes (no false positives).mvn -pl dubbo-serialization/dubbo-serialization-hessian2 -am test: 811 tests, 0 failures.spotless:checkpasses; new patch lines are 100% covered.Checklist