1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
|
From 6ac3c0b7abd35f37201ed2d6298ecef4ea1ae1dd Mon Sep 17 00:00:00 2001
From: "meir@redislabs.com" <meir@redislabs.com>
Date: Sun, 13 Jun 2021 14:29:20 +0300
Subject: [PATCH] Fix protocol parsing on 'ldbReplParseCommand'
(CVE-2021-32672)
The protocol parsing on 'ldbReplParseCommand' (LUA debugging)Assumed protocol correctness. This means that if the following
is given:
*1
$100
test
The parser will try to read additional 94 unallocated bytes after
the client buffer.
This commit fixes this issue by validating that there are actually enough
bytes to read. It also limits the amount of data that can be sent by
the debugger client to 1M so the client will not be able to explode
the memory.
---
src/scripting.c | 29 +++++++++++++++++++++++++----
tests/unit/scripting.tcl | 14 ++++++++++++++
2 files changed, 39 insertions(+), 4 deletions(-)
diff --git a/src/scripting.c b/src/scripting.c
index a781e68..ecb6811 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -1784,7 +1784,8 @@ int ldbDelBreakpoint(int line) {
/* Expect a valid multi-bulk command in the debugging client query buffer.
* On success the command is parsed and returned as an array of SDS strings,
* otherwise NULL is returned and there is to read more buffer. */
-sds *ldbReplParseCommand(int *argcp) {
+sds *ldbReplParseCommand(int *argcp, char** err) {
+ static char* protocol_error = "protocol error";
sds *argv = NULL;
int argc = 0;
if (sdslen(ldb.cbuf) == 0) return NULL;
@@ -1801,7 +1802,7 @@ sds *ldbReplParseCommand(int *argcp) {
/* Seek and parse *<count>\r\n. */
p = strchr(p,'*'); if (!p) goto protoerr;
char *plen = p+1; /* Multi bulk len pointer. */
- p = strstr(p,"\r\n"); if (!p) goto protoerr;
+ p = strstr(p,"\r\n"); if (!p) goto keep_reading;
*p = '\0'; p += 2;
*argcp = atoi(plen);
if (*argcp <= 0 || *argcp > 1024) goto protoerr;
@@ -1810,12 +1811,16 @@ sds *ldbReplParseCommand(int *argcp) {
argv = zmalloc(sizeof(sds)*(*argcp));
argc = 0;
while(argc < *argcp) {
+ // reached the end but there should be more data to read
+ if (*p == '\0') goto keep_reading;
+
if (*p != '$') goto protoerr;
plen = p+1; /* Bulk string len pointer. */
- p = strstr(p,"\r\n"); if (!p) goto protoerr;
+ p = strstr(p,"\r\n"); if (!p) goto keep_reading;
*p = '\0'; p += 2;
int slen = atoi(plen); /* Length of this arg. */
if (slen <= 0 || slen > 1024) goto protoerr;
+ if ((size_t)(p + slen + 2 - copy) > sdslen(copy) ) goto keep_reading;
argv[argc++] = sdsnewlen(p,slen);
p += slen; /* Skip the already parsed argument. */
if (p[0] != '\r' || p[1] != '\n') goto protoerr;
@@ -1825,6 +1830,8 @@ sds *ldbReplParseCommand(int *argcp) {
return argv;
protoerr:
+ *err = protocol_error;
+keep_reading:
sdsfreesplitres(argv,argc);
sdsfree(copy);
return NULL;
@@ -2246,12 +2253,17 @@ void ldbMaxlen(sds *argv, int argc) {
int ldbRepl(lua_State *lua) {
sds *argv;
int argc;
+ char* err = NULL;
/* We continue processing commands until a command that should return
* to the Lua interpreter is found. */
while(1) {
- while((argv = ldbReplParseCommand(&argc)) == NULL) {
+ while((argv = ldbReplParseCommand(&argc, &err)) == NULL) {
char buf[1024];
+ if (err) {
+ lua_pushstring(lua, err);
+ lua_error(lua);
+ }
int nread = read(ldb.fd,buf,sizeof(buf));
if (nread <= 0) {
/* Make sure the script runs without user input since the
@@ -2261,6 +2273,15 @@ int ldbRepl(lua_State *lua) {
return C_ERR;
}
ldb.cbuf = sdscatlen(ldb.cbuf,buf,nread);
+ /* after 1M we will exit with an error
+ * so that the client will not blow the memory
+ */
+ if (sdslen(ldb.cbuf) > 1<<20) {
+ sdsfree(ldb.cbuf);
+ ldb.cbuf = sdsempty();
+ lua_pushstring(lua, "max client buffer reached");
+ lua_error(lua);
+ }
}
/* Flush the old buffer. */
diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl
index be82e15..48c9f1f 100644
--- a/tests/unit/scripting.tcl
+++ b/tests/unit/scripting.tcl
@@ -733,3 +733,17 @@ start_server {tags {"scripting repl"}} {
}
}
+start_server {tags {"scripting needs:debug external:skip"}} {
+ test {Test scripting debug protocol parsing} {
+ r script debug sync
+ r eval {return 'hello'} 0
+ catch {r 'hello\0world'} e
+ assert_match {*Unknown Redis Lua debugger command*} $e
+ catch {r 'hello\0'} e
+ assert_match {*Unknown Redis Lua debugger command*} $e
+ catch {r '\0hello'} e
+ assert_match {*Unknown Redis Lua debugger command*} $e
+ catch {r '\0hello\0'} e
+ assert_match {*Unknown Redis Lua debugger command*} $e
+ }
+}
--
2.27.0
|
