From c68ba74a511eb2ea4b7fa2fb1300c1bad8b1a6a6 Mon Sep 17 00:00:00 2001 From: Tristan Su Date: Wed, 9 Sep 2026 09:28:09 +0800 Subject: [PATCH] Avoid int overflow in sorted-set numkeys argument-count checks Follow-up to #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. --- libs/server/Resp/Objects/SortedSetCommands.cs | 10 ++++---- .../RespSortedSetTests.cs | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/libs/server/Resp/Objects/SortedSetCommands.cs b/libs/server/Resp/Objects/SortedSetCommands.cs index e86bbe6ae00..3b1ecf1e522 100644 --- a/libs/server/Resp/Objects/SortedSetCommands.cs +++ b/libs/server/Resp/Objects/SortedSetCommands.cs @@ -431,7 +431,7 @@ private unsafe bool SortedSetMPop(ref TGarnetApi storageApi) } // Validate we have enough arguments (no of keys + (MIN or MAX)) - if (parseState.Count < numKeys + 2) + if (parseState.Count - 2 < numKeys) { return AbortWithErrorMessage(CmdStrings.RESP_SYNTAX_ERROR); } @@ -1068,7 +1068,7 @@ private unsafe bool SortedSetIntersect(ref TGarnetApi storageApi) return AbortWithErrorMessage(CmdStrings.GenericErrAtLeastOneKey, nameof(RespCommand.ZINTER)); } - if (parseState.Count < nKeys + 1) + if (parseState.Count - 1 < nKeys) { return AbortWithErrorMessage(CmdStrings.RESP_SYNTAX_ERROR); } @@ -1191,7 +1191,7 @@ private unsafe bool SortedSetIntersectLength(ref TGarnetApi storageA return AbortWithErrorMessage(CmdStrings.GenericErrAtLeastOneKey, nameof(RespCommand.ZINTERCARD)); } - if (parseState.Count < nKeys + 1) + if (parseState.Count - 1 < nKeys) { return AbortWithErrorMessage(CmdStrings.RESP_SYNTAX_ERROR); } @@ -1363,7 +1363,7 @@ private unsafe bool SortedSetUnion(ref TGarnetApi storageApi) return AbortWithErrorMessage(CmdStrings.GenericErrAtLeastOneKey, nameof(RespCommand.ZUNION)); } - if (parseState.Count < nKeys + 1) + if (parseState.Count - 1 < nKeys) { return AbortWithErrorMessage(CmdStrings.RESP_SYNTAX_ERROR); } @@ -1493,7 +1493,7 @@ private unsafe bool SortedSetUnionStore(ref TGarnetApi storageApi) return AbortWithErrorMessage(CmdStrings.GenericErrAtLeastOneKey, nameof(RespCommand.ZUNIONSTORE)); } - if (parseState.Count < nKeys + 2) + if (parseState.Count - 2 < nKeys) { return AbortWithErrorMessage(CmdStrings.RESP_SYNTAX_ERROR); } diff --git a/test/standalone/Garnet.test.collections/RespSortedSetTests.cs b/test/standalone/Garnet.test.collections/RespSortedSetTests.cs index c01b38ca9c3..00dd601d910 100644 --- a/test/standalone/Garnet.test.collections/RespSortedSetTests.cs +++ b/test/standalone/Garnet.test.collections/RespSortedSetTests.cs @@ -5347,6 +5347,30 @@ public void CanDoZInterStoreWithBadNumKeysLC() TestUtils.AssertEqualUpToExpectedLength(expectedResponse, response); } + [Test] + public void CanDoSortedSetCommandsWithBadNumKeysLC() + { + using var lightClientRequest = TestUtils.CreateRequest(); + + // Same overflow as CanDoZInterStoreWithBadNumKeysLC, for the sibling commands. + var expectedResponse = $"-{Encoding.ASCII.GetString(CmdStrings.RESP_SYNTAX_ERROR)}\r\n+PONG\r\n"; + + var commands = new[] + { + "ZMPOP 2147483647 zset1 MIN", + "ZUNION 2147483647 zset1", + "ZUNIONSTORE dest 2147483647 zset1", + "ZINTER 2147483647 zset1", + "ZINTERCARD 2147483647 zset1", + }; + + foreach (var command in commands) + { + var response = lightClientRequest.SendCommands(command, "PING"); + TestUtils.AssertEqualUpToExpectedLength(expectedResponse, response); + } + } + [Test] public void ZInterResultOrder() {