Skip to content

Commit 6ac3c0b

Browse files
MeirShpilraienoranagra
authored andcommitted
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.
1 parent 5674b00 commit 6ac3c0b

File tree

2 files changed

+40
-4
lines changed

2 files changed

+40
-4
lines changed

Diff for: src/scripting.c

+25-4
Original file line numberDiff line numberDiff line change
@@ -2026,7 +2026,8 @@ int ldbDelBreakpoint(int line) {
20262026
/* Expect a valid multi-bulk command in the debugging client query buffer.
20272027
* On success the command is parsed and returned as an array of SDS strings,
20282028
* otherwise NULL is returned and there is to read more buffer. */
2029-
sds *ldbReplParseCommand(int *argcp) {
2029+
sds *ldbReplParseCommand(int *argcp, char** err) {
2030+
static char* protocol_error = "protocol error";
20302031
sds *argv = NULL;
20312032
int argc = 0;
20322033
if (sdslen(ldb.cbuf) == 0) return NULL;
@@ -2043,7 +2044,7 @@ sds *ldbReplParseCommand(int *argcp) {
20432044
/* Seek and parse *<count>\r\n. */
20442045
p = strchr(p,'*'); if (!p) goto protoerr;
20452046
char *plen = p+1; /* Multi bulk len pointer. */
2046-
p = strstr(p,"\r\n"); if (!p) goto protoerr;
2047+
p = strstr(p,"\r\n"); if (!p) goto keep_reading;
20472048
*p = '\0'; p += 2;
20482049
*argcp = atoi(plen);
20492050
if (*argcp <= 0 || *argcp > 1024) goto protoerr;
@@ -2052,12 +2053,16 @@ sds *ldbReplParseCommand(int *argcp) {
20522053
argv = zmalloc(sizeof(sds)*(*argcp));
20532054
argc = 0;
20542055
while(argc < *argcp) {
2056+
// reached the end but there should be more data to read
2057+
if (*p == '\0') goto keep_reading;
2058+
20552059
if (*p != '$') goto protoerr;
20562060
plen = p+1; /* Bulk string len pointer. */
2057-
p = strstr(p,"\r\n"); if (!p) goto protoerr;
2061+
p = strstr(p,"\r\n"); if (!p) goto keep_reading;
20582062
*p = '\0'; p += 2;
20592063
int slen = atoi(plen); /* Length of this arg. */
20602064
if (slen <= 0 || slen > 1024) goto protoerr;
2065+
if ((size_t)(p + slen + 2 - copy) > sdslen(copy) ) goto keep_reading;
20612066
argv[argc++] = sdsnewlen(p,slen);
20622067
p += slen; /* Skip the already parsed argument. */
20632068
if (p[0] != '\r' || p[1] != '\n') goto protoerr;
@@ -2067,6 +2072,8 @@ sds *ldbReplParseCommand(int *argcp) {
20672072
return argv;
20682073

20692074
protoerr:
2075+
*err = protocol_error;
2076+
keep_reading:
20702077
sdsfreesplitres(argv,argc);
20712078
sdsfree(copy);
20722079
return NULL;
@@ -2555,12 +2562,17 @@ void ldbMaxlen(sds *argv, int argc) {
25552562
int ldbRepl(lua_State *lua) {
25562563
sds *argv;
25572564
int argc;
2565+
char* err = NULL;
25582566

25592567
/* We continue processing commands until a command that should return
25602568
* to the Lua interpreter is found. */
25612569
while(1) {
2562-
while((argv = ldbReplParseCommand(&argc)) == NULL) {
2570+
while((argv = ldbReplParseCommand(&argc, &err)) == NULL) {
25632571
char buf[1024];
2572+
if (err) {
2573+
lua_pushstring(lua, err);
2574+
lua_error(lua);
2575+
}
25642576
int nread = connRead(ldb.conn,buf,sizeof(buf));
25652577
if (nread <= 0) {
25662578
/* Make sure the script runs without user input since the
@@ -2570,6 +2582,15 @@ int ldbRepl(lua_State *lua) {
25702582
return C_ERR;
25712583
}
25722584
ldb.cbuf = sdscatlen(ldb.cbuf,buf,nread);
2585+
/* after 1M we will exit with an error
2586+
* so that the client will not blow the memory
2587+
*/
2588+
if (sdslen(ldb.cbuf) > 1<<20) {
2589+
sdsfree(ldb.cbuf);
2590+
ldb.cbuf = sdsempty();
2591+
lua_pushstring(lua, "max client buffer reached");
2592+
lua_error(lua);
2593+
}
25732594
}
25742595

25752596
/* Flush the old buffer. */

Diff for: tests/unit/scripting.tcl

+15
Original file line numberDiff line numberDiff line change
@@ -820,3 +820,18 @@ start_server {tags {"scripting"}} {
820820
r eval {return 'hello'} 0
821821
r eval {return 'hello'} 0
822822
}
823+
824+
start_server {tags {"scripting needs:debug external:skip"}} {
825+
test {Test scripting debug protocol parsing} {
826+
r script debug sync
827+
r eval {return 'hello'} 0
828+
catch {r 'hello\0world'} e
829+
assert_match {*Unknown Redis Lua debugger command*} $e
830+
catch {r 'hello\0'} e
831+
assert_match {*Unknown Redis Lua debugger command*} $e
832+
catch {r '\0hello'} e
833+
assert_match {*Unknown Redis Lua debugger command*} $e
834+
catch {r '\0hello\0'} e
835+
assert_match {*Unknown Redis Lua debugger command*} $e
836+
}
837+
}

0 commit comments

Comments
 (0)