This is an automated email from the ASF dual-hosted git repository.
git-hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new d69035320 fix(commands): fix out-of-bounds access in CLIENT KILL
argument parsing (#3661)
d69035320 is described below
commit d69035320ca27f3226c71cf09acdb25efbe60e82
Author: weimeng <[email protected]>
AuthorDate: Wed Oct 14 16:41:18 2026 +0800
fix(commands): fix out-of-bounds access in CLIENT KILL argument parsing
(#3661)
## Summary
Fix a bounds-check bug in `CLIENT KILL` new-format argument parsing
where a
keyword at the end of the argument list with no following value could
lead to
an out-of-bounds read instead of returning a syntax error.
## Root Cause
In `src/commands/cmd_server.cc`, the `ClientKillSubCommand::Parse`
method uses
a `more_args` guard to check whether a value follows each keyword before
accessing `args[i + 1]`:
```cpp
bool more_args = i < args.size();
```
## AI Usage Disclosure
- The bug fix was implemented manually.
- AI tools(Qoder) were used to assist in generating unit test
scaffolding; all tests were reviewed and verified by the author.
---------
Co-authored-by: weimeng <[email protected]>
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
Co-authored-by: Aleks Lozovyuk <[email protected]>
---
src/commands/cmd_server.cc | 2 +-
tests/gocase/unit/client/client_kill_test.go | 70 ++++++++++++++++++++++++++++
2 files changed, 71 insertions(+), 1 deletion(-)
diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc
index b57382b53..ac1fd2561 100644
--- a/src/commands/cmd_server.cc
+++ b/src/commands/cmd_server.cc
@@ -610,7 +610,7 @@ class CommandClient : public Commander {
new_format_ = true;
while (i < args.size()) {
- bool more_args = i < args.size();
+ bool more_args = i + 1 < args.size();
if (util::EqualICase(args[i], "addr") && more_args) {
addr_ = args[i + 1];
} else if (util::EqualICase(args[i], "id") && more_args) {
diff --git a/tests/gocase/unit/client/client_kill_test.go
b/tests/gocase/unit/client/client_kill_test.go
new file mode 100644
index 000000000..789d1043f
--- /dev/null
+++ b/tests/gocase/unit/client/client_kill_test.go
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package client
+
+import (
+ "context"
+ "strings"
+ "testing"
+
+ "github.com/apache/kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func TestClientKillParseEdgeCases(t *testing.T) {
+ srv := util.StartServer(t, map[string]string{})
+ defer srv.Close()
+
+ ctx := context.Background()
+ rdb := srv.NewClient()
+ defer func() { require.NoError(t, rdb.Close()) }()
+
+ t.Run("CLIENT KILL with keyword but missing value returns error",
func(t *testing.T) {
+ keywords := []string{"addr", "id", "skipme", "type"}
+ for _, kw := range keywords {
+ err := rdb.Do(ctx, "CLIENT", "KILL", "addr",
"127.0.0.1:1234", kw).Err()
+ require.ErrorContains(t, err, "syntax", "CLIENT KILL %s
should fail with a syntax error", kw)
+ }
+ })
+
+ t.Run("CLIENT KILL with odd number of new-format args returns syntax
error", func(t *testing.T) {
+ err := rdb.Do(ctx, "CLIENT", "KILL", "addr", "127.0.0.1:1234",
"type").Err()
+ require.Error(t, err)
+ require.True(t, strings.Contains(err.Error(), "syntax") ||
strings.Contains(err.Error(), "Syntax"),
+ "expected syntax error, got: %s", err.Error())
+ })
+
+ t.Run("CLIENT KILL with unknown option returns syntax error", func(t
*testing.T) {
+ err := rdb.Do(ctx, "CLIENT", "KILL", "unknown", "value").Err()
+ require.Error(t, err)
+ require.True(t, strings.Contains(err.Error(), "syntax") ||
strings.Contains(err.Error(), "Syntax"),
+ "expected syntax error, got: %s", err.Error())
+ })
+
+ t.Run("CLIENT KILL with valid new-format options succeeds", func(t
*testing.T) {
+ err := rdb.Do(ctx, "CLIENT", "KILL", "addr",
"127.0.0.1:1234").Err()
+ require.NoError(t, err)
+ })
+
+ t.Run("CLIENT KILL with multiple valid options succeeds", func(t
*testing.T) {
+ err := rdb.Do(ctx, "CLIENT", "KILL", "type", "normal",
"skipme", "yes").Err()
+ require.NoError(t, err)
+ })
+}
