133 lines
4.8 KiB
Diff
133 lines
4.8 KiB
Diff
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
|
|
|