CVE-2023-28425: Improper Neutralization of Special Elements used in a Command ('Command Injection') (CWE-77)
value: Redis is an in-memory database that persists on disk. Starting in version 7.0.8 and prior to version 7.0.10, authenticated users can use the MSETNX command to trigger a runtime assertion and termination of the Redis server process. The problem is fixed in Redis version 7.0.10.
CWE description: The product constructs all or part of a command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended command when it is sent to a downstream component.
CVSS v3 base score: 5.5
Published: 2023-03-20T20:15Z
Repository: https://github.com/redis/redis
Language: Other
Files changed: src/t_string.c, tests/unit/type/string.tcl
Commit date: 2023-03-20 18:50:44 +0200
Commit message: Avoid assertion when MSETNX is used with the same key twice (CVE-2023-28425) (#11940)

Using the same key twice in MSETNX command would trigger an assertion.

This reverts #11594 (introduced in Redis 7.0.8)
Diff: diff --git a/src/t_string.c b/src/t_string.c
index 6208d6a853..96bbf606db 100644
--- a/src/t_string.c
+++ b/src/t_string.c
@@ -557,11 +557,10 @@ void mgetCommand(client *c) {
     }
 }
 
 void msetGenericCommand(client *c, int nx) {
     int j;
-    int setkey_flags = 0;
 
     if ((c->argc % 2) == 0) {
         addReplyErrorArity(c);
         return;
     }
@@ -573,16 +572,15 @@ void msetGenericCommand(client *c, int nx) {
             if (lookupKeyWrite(c->db,c->argv[j]) != NULL) {
                 addReply(c, shared.czero);
                 return;
             }
         }
-        setkey_flags |= SETKEY_DOESNT_EXIST;
     }
 
     for (j = 1; j < c->argc; j += 2) {
         c->argv[j+1] = tryObjectEncoding(c->argv[j+1]);
-        setKey(c, c->db, c->argv[j], c->argv[j + 1], setkey_flags);
+        setKey(c, c->db, c->argv[j], c->argv[j + 1], 0);
         notifyKeyspaceEvent(NOTIFY_STRING,"set",c->argv[j],c->db->id);
     }
     server.dirty += (c->argc-1)/2;
     addReply(c, nx ? shared.cone : shared.ok);
 }
diff --git a/tests/unit/type/string.tcl b/tests/unit/type/string.tcl
index a9fa894dcc..b25a14f4c3 100644
--- a/tests/unit/type/string.tcl
+++ b/tests/unit/type/string.tcl
@@ -232,10 +232,19 @@ start_server {tags {"string"}} {
 
     test {MSETNX with not existing keys} {
         list [r msetnx x1{t} xxx y2{t} yyy] [r get x1{t}] [r get y2{t}]
     } {1 xxx yyy}
 
+    test {MSETNX with not existing keys - same key twice} {
+        r del x1{t}
+        list [r msetnx x1{t} xxx x1{t} yyy] [r get x1{t}]
+    } {1 yyy}
+
+    test {MSETNX with already existing keys - same key twice} {
+        list [r msetnx x1{t} xxx x1{t} zzz] [r get x1{t}]
+    } {0 yyy}
+
     test "STRLEN against non-existing key" {
         assert_equal 0 [r strlen notakey]
     }
 
     test "STRLEN against integer-encoded value" {

Vulnerable code:     int setkey_flags = 0;
        setkey_flags |= SETKEY_DOESNT_EXIST;
        setKey(c, c->db, c->argv[j], c->argv[j + 1], setkey_flags);
Fixed code:         setKey(c, c->db, c->argv[j], c->argv[j + 1], 0);
    test {MSETNX with not existing keys - same key twice} {
        r del x1{t}
        list [r msetnx x1{t} xxx x1{t} yyy] [r get x1{t}]
    } {1 yyy}

    test {MSETNX with already existing keys - same key twice} {
        list [r msetnx x1{t} xxx x1{t} zzz] [r get x1{t}]
    } {0 yyy}