(kvrocks) branch unstable updated: fix(string): the server will be crashed when passing a negative offset in SETRANGE command (#2783)

[hu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

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 047490ee fix(string): the server will be crashed when passing a negative offset in SETRANGE command (#2783)
047490ee is described below

commit 047490ee93122bee279e51305bb856ae74fba60a
Author: hulk <hu...@gmail.com>
AuthorDate: Mon Feb 10 23:38:37 2025 +0800

    fix(string): the server will be crashed when passing a negative offset in SETRANGE command (#2783)
---
 src/commands/cmd_string.cc                     |  7 ++++++-
 tests/gocase/unit/type/strings/strings_test.go | 17 +++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/src/commands/cmd_string.cc b/src/commands/cmd_string.cc
index cfeb10ef..5b40b47a 100644
--- a/src/commands/cmd_string.cc
+++ b/src/commands/cmd_string.cc
@@ -221,7 +221,7 @@ class CommandSubStr : public CommandGetRange {
 class CommandSetRange : public Commander {
  public:
   Status Parse(const std::vector<std::string> &args) override {
-    auto parse_result = ParseInt<int>(args[2], 10);
+    auto parse_result = ParseInt<int>(args[2], {0, INT32_MAX}, 10);
     if (!parse_result) {
       return {Status::RedisParseErr, errValueNotInteger};
     }
@@ -234,6 +234,11 @@ class CommandSetRange : public Commander {
     uint64_t ret = 0;
     redis::String string_db(srv->storage, conn->GetNamespace());
 
+    auto total = offset_ + args_[3].size();
+    if (total > srv->GetConfig()->proto_max_bulk_len) {
+      return {Status::RedisExecErr, "string exceeds maximum allowed size"};
+    }
+
     auto s = string_db.SetRange(ctx, args_[1], offset_, args_[3], &ret);
     if (!s.ok()) {
       return {Status::RedisExecErr, s.ToString()};
diff --git a/tests/gocase/unit/type/strings/strings_test.go b/tests/gocase/unit/type/strings/strings_test.go
index e5938182..305ab397 100644
--- a/tests/gocase/unit/type/strings/strings_test.go
+++ b/tests/gocase/unit/type/strings/strings_test.go
@@ -500,6 +500,23 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) {
 		require.ErrorContains(t, rdb.SetRange(ctx, "mykey", 0, "bar").Err(), "WRONGTYPE")
 	})
 
+	t.Run("SETRANGE with negative offset", func(t *testing.T) {
+		require.ErrorContains(t, rdb.SetRange(ctx, "setrange_negative_offset", -1, "bar").Err(),
+			"value is not an integer or out of range")
+		require.ErrorContains(t, rdb.SetRange(ctx, "setrange_negative_offset", -2147483599, "bar").Err(),
+			"value is not an integer or out of range")
+	})
+
+	t.Run("SETRANGE with offset + value length too large", func(t *testing.T) {
+		protoMaxBulkLen := int64(1024 * 1024)
+		require.NoError(t, rdb.ConfigSet(ctx, "proto-max-bulk-len", strconv.FormatInt(protoMaxBulkLen, 10)).Err())
+		require.ErrorContains(t, rdb.SetRange(ctx, "setrange_out_of_range", protoMaxBulkLen, "world").Err(),
+			"string exceeds maximum allowed size")
+
+		// it should be able to set the value if the length is protoMaxBulkLen
+		require.NoError(t, rdb.SetRange(ctx, "setrange_out_of_range", protoMaxBulkLen-5, "world").Err())
+	})
+
 	t.Run("GETRANGE against non-existing key", func(t *testing.T) {
 		require.NoError(t, rdb.Del(ctx, "mykey").Err())
 		require.EqualValues(t, "", rdb.GetRange(ctx, "mykey", 0, -1).Val())