Skip to content

fix(serialization): enforce writeReplace security check for hessian2 - #16437

Open
GerardGao wants to merge 2 commits into
apache:3.2from
GerardGao:fix/issue-16287-hessian2-writereplace-check
Open

fix(serialization): enforce writeReplace security check for hessian2#16437
GerardGao wants to merge 2 commits into
apache:3.2from
GerardGao:fix/issue-16287-hessian2-writereplace-check

Conversation

@GerardGao

@GerardGao GerardGao commented Aug 26, 2026

Copy link
Copy Markdown

What is the purpose of the change

hessian-lite 3.2.x resolves classes that declare a writeReplace() method directly to JavaSerializer, before getDefaultSerializer() is ever consulted. Such classes therefore skipped the checkSerializable() security check entirely: a class that does not implement Serializable could be serialized as long as it declared writeReplace() (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 writeReplace branch).

Brief changelog

  • Hessian2SerializerFactory registers a pre-flight AbstractSerializerFactory via addFactory that applies the existing checkSerializable() to classes carrying a writeReplace() method, then returns null so the normal SerializerFactory resolution chain keeps control.
  • The check mirrors the behavior introduced for writeReplace classes in hessian-lite 4.x, scoped to the 3.2 line.

Verifying this change

  • New Hessian2WriteReplaceSecurityTest:
    • a non-Serializable class with writeReplace() is rejected;
    • a Serializable class whose writeReplace() returns a non-Serializable holder is rejected;
    • a Serializable class whose writeReplace() returns a String still serializes (no false positives).
  • mvn -pl dubbo-serialization/dubbo-serialization-hessian2 -am test: 811 tests, 0 failures.
  • spotless:check passes; new patch lines are 100% covered.

Checklist

  • Make sure there is a GitHub_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GitHub issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
  • Add some description to dubbo-website project if you are requesting to add a feature.
  • GitHub Actions works fine on your own branch.
  • If this contribution is large, please follow the Software Donation Guide.

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
@GerardGao

Copy link
Copy Markdown
Author

Note on CI: the "Build and Test For PR" workflow exits with startup_failure before any job is created on every recent run against 3.2 — including the maintainer push that prepared the 3.2.20 release (run 31576523922, 2026-08-12) — while January runs on the same branch succeeded. This looks like a repository-level workflow issue on the 3.2 branch rather than a problem with this change.

Local verification for this PR:

  • mvn -pl dubbo-serialization/dubbo-serialization-hessian2 -am test: 811 tests, 0 failures
  • spotless:check: passes
  • The new test reproduces the reported bypass before the fix and passes after it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant