Avoid int overflow in sorted-set numkeys argument-count checks - #2112
Open
Tristan Su (foobar) wants to merge 1 commit into
Open
Avoid int overflow in sorted-set numkeys argument-count checks#2112Tristan Su (foobar) wants to merge 1 commit into
Tristan Su (foobar) wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused validation changes are correct and covered by appropriate regression tests.
Pull request overview
Prevents integer overflow in sorted-set numkeys validation, avoiding out-of-bounds parsing and connection termination.
Changes:
- Replaces overflow-prone addition checks with safe subtraction checks.
- Adds regression coverage for five affected commands and verifies connection survival.
File summaries
| File | Description |
|---|---|
libs/server/Resp/Objects/SortedSetCommands.cs |
Safely validates argument counts for five sorted-set commands. |
test/standalone/Garnet.test.collections/RespSortedSetTests.cs |
Tests int.MaxValue inputs and subsequent PING responses. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Author
|
@microsoft-github-policy-service agree |
Follow-up to microsoft#2031, which hardened ZINTERSTORE's guard as 'parseState.Count - 2 < nKeys' but left the additive form in five sibling handlers. A numkeys near int.MaxValue wraps the check negative, so a single command passes validation and reads parseState out of bounds: ZMPOP 2147483647 key MIN, ZUNION 2147483647 key, ZUNIONSTORE dst 2147483646 key -> GetArgSliceByRef at a wild index (process crash) ZINTER / ZINTERCARD -> Span.Slice throw (connection drop) Use the subtraction form everywhere. Behavior is unchanged whenever the addition did not overflow.
Tristan Su (foobar)
force-pushed
the
fix/numkeys-overflow-zmpop-zunion
branch
from
September 9, 2026 01:58
d1eb1e9 to
c68ba74
Compare
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.
Follow-up to #2031, which fixed this for ZINTERSTORE. The same additive
argument-count checks remained in ZMPOP, ZUNION, ZUNIONSTORE, ZINTER, and
ZINTERCARD — a numkeys near
int.MaxValuewraps the check negative andslips past, so a single command reads the parse state out of bounds:
Repro on a default deployment:
ZMPOP 2147483647 k MIN.This PR applies the same subtraction form to all five guards. Behavior is
unchanged whenever the addition did not overflow. Adds
CanDoSortedSetCommandsWithBadNumKeysLC, mirroringCanDoZInterStoreWithBadNumKeysLC: each command withnumkeys = 2147483647followed byPINGmust get a clean syntax errorwith the connection intact.