Commit f621c0a4 authored by meir@redislabs.com's avatar meir@redislabs.com Committed by Oran Agra
Browse files

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.

(cherry picked from commit 4f95e6809902665ced50646107d174dd8137bcaa)
parent 71be9729
...@@ -1829,7 +1829,8 @@ int ldbDelBreakpoint(int line) { ...@@ -1829,7 +1829,8 @@ int ldbDelBreakpoint(int line) {
/* Expect a valid multi-bulk command in the debugging client query buffer. /* 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, * 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. */ * 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; sds *argv = NULL;
int argc = 0; int argc = 0;
if (sdslen(ldb.cbuf) == 0) return NULL; if (sdslen(ldb.cbuf) == 0) return NULL;
...@@ -1846,7 +1847,7 @@ sds *ldbReplParseCommand(int *argcp) { ...@@ -1846,7 +1847,7 @@ sds *ldbReplParseCommand(int *argcp) {
/* Seek and parse *<count>\r\n. */ /* Seek and parse *<count>\r\n. */
p = strchr(p,'*'); if (!p) goto protoerr; p = strchr(p,'*'); if (!p) goto protoerr;
char *plen = p+1; /* Multi bulk len pointer. */ 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; *p = '\0'; p += 2;
*argcp = atoi(plen); *argcp = atoi(plen);
if (*argcp <= 0 || *argcp > 1024) goto protoerr; if (*argcp <= 0 || *argcp > 1024) goto protoerr;
...@@ -1855,12 +1856,16 @@ sds *ldbReplParseCommand(int *argcp) { ...@@ -1855,12 +1856,16 @@ sds *ldbReplParseCommand(int *argcp) {
argv = zmalloc(sizeof(sds)*(*argcp)); argv = zmalloc(sizeof(sds)*(*argcp));
argc = 0; argc = 0;
while(argc < *argcp) { while(argc < *argcp) {
// reached the end but there should be more data to read
if (*p == '\0') goto keep_reading;
if (*p != '$') goto protoerr; if (*p != '$') goto protoerr;
plen = p+1; /* Bulk string len pointer. */ 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; *p = '\0'; p += 2;
int slen = atoi(plen); /* Length of this arg. */ int slen = atoi(plen); /* Length of this arg. */
if (slen <= 0 || slen > 1024) goto protoerr; if (slen <= 0 || slen > 1024) goto protoerr;
if ((size_t)(p + slen + 2 - copy) > sdslen(copy) ) goto keep_reading;
argv[argc++] = sdsnewlen(p,slen); argv[argc++] = sdsnewlen(p,slen);
p += slen; /* Skip the already parsed argument. */ p += slen; /* Skip the already parsed argument. */
if (p[0] != '\r' || p[1] != '\n') goto protoerr; if (p[0] != '\r' || p[1] != '\n') goto protoerr;
...@@ -1870,6 +1875,8 @@ sds *ldbReplParseCommand(int *argcp) { ...@@ -1870,6 +1875,8 @@ sds *ldbReplParseCommand(int *argcp) {
return argv; return argv;
protoerr: protoerr:
*err = protocol_error;
keep_reading:
sdsfreesplitres(argv,argc); sdsfreesplitres(argv,argc);
sdsfree(copy); sdsfree(copy);
return NULL; return NULL;
...@@ -2292,12 +2299,17 @@ void ldbMaxlen(sds *argv, int argc) { ...@@ -2292,12 +2299,17 @@ void ldbMaxlen(sds *argv, int argc) {
int ldbRepl(lua_State *lua) { int ldbRepl(lua_State *lua) {
sds *argv; sds *argv;
int argc; int argc;
char* err = NULL;
/* We continue processing commands until a command that should return /* We continue processing commands until a command that should return
* to the Lua interpreter is found. */ * to the Lua interpreter is found. */
while(1) { while(1) {
while((argv = ldbReplParseCommand(&argc)) == NULL) { while((argv = ldbReplParseCommand(&argc, &err)) == NULL) {
char buf[1024]; char buf[1024];
if (err) {
lua_pushstring(lua, err);
lua_error(lua);
}
int nread = read(ldb.fd,buf,sizeof(buf)); int nread = read(ldb.fd,buf,sizeof(buf));
if (nread <= 0) { if (nread <= 0) {
/* Make sure the script runs without user input since the /* Make sure the script runs without user input since the
...@@ -2307,6 +2319,15 @@ int ldbRepl(lua_State *lua) { ...@@ -2307,6 +2319,15 @@ int ldbRepl(lua_State *lua) {
return C_ERR; return C_ERR;
} }
ldb.cbuf = sdscatlen(ldb.cbuf,buf,nread); 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. */ /* Flush the old buffer. */
......
...@@ -741,3 +741,17 @@ start_server {tags {"scripting repl"}} { ...@@ -741,3 +741,17 @@ start_server {tags {"scripting repl"}} {
} }
} }
start_server {tags {"scripting"}} {
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
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment