Skip to content

fix(go): add bound checking for refResolver and metaStringResolver reads#3615

Merged
chaokunyang merged 3 commits intoapache:mainfrom
ayush00git:fix/resolve_refTrackId
Apr 24, 2026
Merged

fix(go): add bound checking for refResolver and metaStringResolver reads#3615
chaokunyang merged 3 commits intoapache:mainfrom
ayush00git:fix/resolve_refTrackId

Conversation

@ayush00git
Copy link
Copy Markdown
Contributor

@ayush00git ayush00git commented Apr 24, 2026

Why?

This PR addresses two critical security vulnerabilities in the Fory Go runtime.
Both issues stemmed from trusting integer values (references/indices) read directly from the wire. A maliciously crafted payload could provide an out-of-bounds index (either too large or negative), triggering a runtime panic. In a server environment, this would allow an attacker to perform a Denial of Service (DoS) attack by crashing the Go process with a single malicious packet.

What does this PR do?

  • Bounds Checking in RefResolver: added an upper-bound check in GetReadObject to ensure refId does not exceed the size of the readObjects slice.
  • Bounds Checking in MetaStringResolver: added a lower-bound check in ReadMetaStringBytes to prevent negative index panics (e.g., when a header value of 1 is provided).
  • Security & Regression Tests:
    Added unit tests in ref_resolver_test.go and meta_string_resolver_test.go that specifically reproduce the OOB and negative-index panics.
  • Added boundary regression tests to verify that valid edge cases (indices 0 and len-1) still function correctly.

Related issues

Closes #3614

AI Contribution Checklist

  • Substantial AI assistance was used in this PR: yes / no
  • If yes, I included a completed AI Contribution Checklist in this PR description and the required AI Usage Disclosure.
  • If yes, my PR description includes the required ai_review summary and screenshot evidence of the final clean AI review results from both fresh reviewers on the current PR diff or current HEAD after the latest code changes.

Does this PR introduce any user-facing change?

  • Does this PR introduce any public API change?
  • Does this PR introduce any binary protocol compatibility change?

Benchmark

N/A

@ayush00git ayush00git requested a review from chaokunyang as a code owner April 24, 2026 10:40
@ayush00git ayush00git changed the title Fix/resolve ref track fix(go): add bound checking for refResolver and metaStringResolver reads Apr 24, 2026
Copy link
Copy Markdown
Collaborator

@chaokunyang chaokunyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chaokunyang chaokunyang merged commit a3845c3 into apache:main Apr 24, 2026
62 checks passed
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.

[Go] out of bounds panics in RefResolver and MetaStringResolver

2 participants