Skip to content

fix(go): add configurable fieldCount and fieldDepth guardrails#3620

Merged
chaokunyang merged 5 commits into
apache:mainfrom
ayush00git:fix/fieldcount_checks
Apr 27, 2026
Merged

fix(go): add configurable fieldCount and fieldDepth guardrails#3620
chaokunyang merged 5 commits into
apache:mainfrom
ayush00git:fix/fieldcount_checks

Conversation

@ayush00git
Copy link
Copy Markdown
Contributor

@ayush00git ayush00git commented Apr 25, 2026

Why?

Malicious payloads could specify a massive fieldCount, causing the runtime to attempt an unbounded memory allocation.
Deeply nested schema definitions (like a LIST of LIST...) could trigger unbounded recursion, exceeding the goroutine stack limit and crashing the process.

What does this PR do?

Added a hard limit of 10,000 fields and a buffer-remaining check in decodeTypeDef to prevent massive slice allocations.
Added a depth parameter to readFieldType and readFieldTypeWithFlags, capping nested schema definitions at a maximum depth of 64.

Related issues

Closes #3619

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

@ayush00git ayush00git requested a review from chaokunyang as a code owner April 25, 2026 21:27
@ayush00git
Copy link
Copy Markdown
Contributor Author

@chaokunyang i'm planning to create these bounds configurable, moving them to the Config.

@ayush00git
Copy link
Copy Markdown
Contributor Author

@chaokunyang have a look

@ayush00git ayush00git changed the title fix(go): added fieldcount boundation checks fix(go): add configurable fieldCount and fieldDepth guardrails Apr 27, 2026
Comment thread go/fory/fory.go Outdated
MaxCollectionSize: 1_000_000,
MaxBinarySize: 64 * 1024 * 1024,
MaxTypeDefFields: 10000,
MaxTypeDefDepth: 64,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You can reuse MaxDepth, we'd better to kepp config options minimal

Comment thread go/fory/fory.go Outdated
IsXlang: false,
MaxCollectionSize: 1_000_000,
MaxBinarySize: 64 * 1024 * 1024,
MaxTypeDefFields: 10000,
Copy link
Copy Markdown
Collaborator

@chaokunyang chaokunyang Apr 27, 2026

Choose a reason for hiding this comment

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

Suggested change
MaxTypeDefFields: 10000,
MaxTypeFields: 10000,

@ayush00git ayush00git requested a review from chaokunyang April 27, 2026 10:45
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 3b68521 into apache:main Apr 27, 2026
62 checks passed
@ayush00git ayush00git deleted the fix/fieldcount_checks branch April 27, 2026 12:29
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] unbounded allocation and recursion in go typedef deserialization

2 participants