Chris Lamb wrote: > > redis: multiple security issues in Lua scripting > > This has now been assigned CVE-2018-11219 & CVE-2018-11218.
Security team, oermission to upload the attached to stretch-security? redis (3:3.2.6-3+deb9u1) stretch-security; urgency=high * CVE-2018-11218, CVE-2018-11219: Backport patches to fix multiple heap corruption and integer overflow vulnerabilities. (Closes: #901495) -- Chris Lamb <la...@debian.org> Thu, 14 Jun 2018 15:08:27 +0200 Regards, -- ,''`. : :' : Chris Lamb `. `'` la...@debian.org / chris-lamb.co.uk `-
diff --git a/debian/changelog b/debian/changelog index c66f34fb..1c449909 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,10 @@ +redis (3:3.2.6-3+deb9u1) stretch-security; urgency=high + + * CVE-2018-11218, CVE-2018-11219: Backport patches to fix multiple heap + corruption and integer overflow vulnerabilities. (Closes: #901495) + + -- Chris Lamb <la...@debian.org> Thu, 14 Jun 2018 15:08:27 +0200 + redis (3:3.2.6-1) unstable; urgency=medium * New upstream release. diff --git a/debian/patches/0009-Security-fix-redis-cli-buffer-overflow.patch b/debian/patches/0009-Security-fix-redis-cli-buffer-overflow.patch new file mode 100644 index 00000000..0ba4ccd7 --- /dev/null +++ b/debian/patches/0009-Security-fix-redis-cli-buffer-overflow.patch @@ -0,0 +1,53 @@ +From: antirez <anti...@gmail.com> +Date: Mon, 11 Jun 2018 12:08:42 +0200 +Subject: Security: fix redis-cli buffer overflow. + +Thanks to Fakhri Zulkifli for reporting it. + +The fix switched to dynamic allocation, copying the final prompt in the +static buffer only at the end. +--- + src/redis-cli.c | 27 ++++++++++++++++----------- + 1 file changed, 16 insertions(+), 11 deletions(-) + +diff --git a/src/redis-cli.c b/src/redis-cli.c +index 9043915..f5e26d9 100644 +--- a/src/redis-cli.c ++++ b/src/redis-cli.c +@@ -151,20 +151,25 @@ static long long mstime(void) { + } + + static void cliRefreshPrompt(void) { +- int len; +- + if (config.eval_ldb) return; +- if (config.hostsocket != NULL) +- len = snprintf(config.prompt,sizeof(config.prompt),"redis %s", +- config.hostsocket); +- else +- len = anetFormatAddr(config.prompt, sizeof(config.prompt), +- config.hostip, config.hostport); ++ ++ sds prompt = sdsempty(); ++ if (config.hostsocket != NULL) { ++ prompt = sdscatfmt(prompt,"redis %s",config.hostsocket); ++ } else { ++ char addr[256]; ++ anetFormatAddr(addr, sizeof(addr), config.hostip, config.hostport); ++ prompt = sdscatlen(prompt,addr,strlen(addr)); ++ } ++ + /* Add [dbnum] if needed */ + if (config.dbnum != 0) +- len += snprintf(config.prompt+len,sizeof(config.prompt)-len,"[%d]", +- config.dbnum); +- snprintf(config.prompt+len,sizeof(config.prompt)-len,"> "); ++ prompt = sdscatfmt(prompt,"[%i]",config.dbnum); ++ ++ /* Copy the prompt in the static buffer. */ ++ prompt = sdscatlen(prompt,"> ",2); ++ snprintf(config.prompt,sizeof(config.prompt),"%s",prompt); ++ sdsfree(prompt); + } + + /* Return the name of the dotfile for the specified 'dotfilename'. diff --git a/debian/patches/0010-Security-fix-Lua-struct-package-offset-handling.patch b/debian/patches/0010-Security-fix-Lua-struct-package-offset-handling.patch new file mode 100644 index 00000000..8cbc89ce --- /dev/null +++ b/debian/patches/0010-Security-fix-Lua-struct-package-offset-handling.patch @@ -0,0 +1,40 @@ +From: antirez <anti...@gmail.com> +Date: Tue, 15 May 2018 13:13:49 +0200 +Subject: Security: fix Lua struct package offset handling. + +After the first fix to the struct package I found another similar +problem, which is fixed by this patch. It could be reproduced easily by +running the following script: + + return struct.unpack('f', "xxxxxxxxxxxxx",-3) + +The above will access bytes before the 'data' pointer. +--- + deps/lua/src/lua_struct.c | 8 ++++++-- + 1 file changed, 6 insertions(+), 2 deletions(-) + +diff --git a/deps/lua/src/lua_struct.c b/deps/lua/src/lua_struct.c +index a602bb4..e263f8e 100644 +--- a/deps/lua/src/lua_struct.c ++++ b/deps/lua/src/lua_struct.c +@@ -295,14 +295,18 @@ static int b_unpack (lua_State *L) { + const char *fmt = luaL_checkstring(L, 1); + size_t ld; + const char *data = luaL_checklstring(L, 2, &ld); +- size_t pos = luaL_optinteger(L, 3, 1) - 1; ++ size_t pos = luaL_optinteger(L, 3, 1); ++ luaL_argcheck(L, pos > 0, 3, "offset must be 1 or greater"); ++ pos--; /* Lua indexes are 1-based, but here we want 0-based for C ++ * pointer math. */ + defaultoptions(&h); + lua_settop(L, 2); + while (*fmt) { + int opt = *fmt++; + size_t size = optsize(L, opt, &fmt); + pos += gettoalign(pos, &h, opt, size); +- luaL_argcheck(L, pos+size <= ld, 2, "data string too short"); ++ luaL_argcheck(L, size <= ld && pos <= ld - size, ++ 2, "data string too short"); + luaL_checkstack(L, 1, "too many results"); + switch (opt) { + case 'b': case 'B': case 'h': case 'H': diff --git a/debian/patches/0011-Security-more-cmsgpack-fixes-by-soloestoy.patch b/debian/patches/0011-Security-more-cmsgpack-fixes-by-soloestoy.patch new file mode 100644 index 00000000..cf0bc3cc --- /dev/null +++ b/debian/patches/0011-Security-more-cmsgpack-fixes-by-soloestoy.patch @@ -0,0 +1,58 @@ +From: antirez <anti...@gmail.com> +Date: Tue, 15 May 2018 12:29:56 +0200 +Subject: Security: more cmsgpack fixes by @soloestoy. + +@soloestoy sent me this additional fixes, after searching for similar +problems to the one reported in mp_pack(). I'm committing the changes +because it was not possible during to make a public PR to protect Redis +users and give Redis providers some time to patch their systems. +--- + deps/lua/src/lua_cmsgpack.c | 7 +++++++ + 1 file changed, 7 insertions(+) + +diff --git a/deps/lua/src/lua_cmsgpack.c b/deps/lua/src/lua_cmsgpack.c +index 90a388f..1a5749c 100644 +--- a/deps/lua/src/lua_cmsgpack.c ++++ b/deps/lua/src/lua_cmsgpack.c +@@ -385,6 +385,7 @@ void mp_encode_lua_table_as_array(lua_State *L, mp_buf *buf, int level) { + #endif + + mp_encode_array(L,buf,len); ++ luaL_checkstack(L, 1, "in function mp_encode_lua_table_as_array"); + for (j = 1; j <= len; j++) { + lua_pushnumber(L,j); + lua_gettable(L,-2); +@@ -400,6 +401,7 @@ void mp_encode_lua_table_as_map(lua_State *L, mp_buf *buf, int level) { + * Lua API, we need to iterate a first time. Note that an alternative + * would be to do a single run, and then hack the buffer to insert the + * map opcodes for message pack. Too hackish for this lib. */ ++ luaL_checkstack(L, 3, "in function mp_encode_lua_table_as_map"); + lua_pushnil(L); + while(lua_next(L,-2)) { + lua_pop(L,1); /* remove value, keep key for next iteration. */ +@@ -519,6 +521,7 @@ int mp_pack(lua_State *L) { + for(i = 1; i <= nargs; i++) { + /* Copy argument i to top of stack for _encode processing; + * the encode function pops it from the stack when complete. */ ++ luaL_checkstack(L, 1, "in function mp_check"); + lua_pushvalue(L, i); + + mp_encode_lua_type(L,buf,0); +@@ -547,6 +550,7 @@ void mp_decode_to_lua_array(lua_State *L, mp_cur *c, size_t len) { + int index = 1; + + lua_newtable(L); ++ luaL_checkstack(L, 1, "in function mp_decode_to_lua_array"); + while(len--) { + lua_pushnumber(L,index++); + mp_decode_to_lua_type(L,c); +@@ -821,6 +825,9 @@ int mp_unpack_full(lua_State *L, int limit, int offset) { + * subtract the entire buffer size from the unprocessed size + * to get our next start offset */ + int offset = len - c.left; ++ ++ luaL_checkstack(L, 1, "in function mp_unpack_full"); ++ + /* Return offset -1 when we have have processed the entire buffer. */ + lua_pushinteger(L, c.left == 0 ? -1 : offset); + /* Results are returned with the arg elements still diff --git a/debian/patches/0012-Security-update-Lua-struct-package-for-security.patch b/debian/patches/0012-Security-update-Lua-struct-package-for-security.patch new file mode 100644 index 00000000..26d1fa40 --- /dev/null +++ b/debian/patches/0012-Security-update-Lua-struct-package-for-security.patch @@ -0,0 +1,103 @@ +From: antirez <anti...@gmail.com> +Date: Mon, 14 May 2018 17:49:06 +0200 +Subject: Security: update Lua struct package for security. + +During an auditing Apple found that the "struct" Lua package +we ship with Redis (http://www.inf.puc-rio.br/~roberto/struct/) contains +a security problem. A bound-checking statement fails because of integer +overflow. The bug exists since we initially integrated this package with +Lua, when scripting was introduced, so every version of Redis with +EVAL/EVALSHA capabilities exposed is affected. + +Instead of just fixing the bug, the library was updated to the latest +version shipped by the author. +--- + deps/lua/src/lua_struct.c | 28 +++++++++++++++------------- + 1 file changed, 15 insertions(+), 13 deletions(-) + +diff --git a/deps/lua/src/lua_struct.c b/deps/lua/src/lua_struct.c +index e263f8e..953c39b 100644 +--- a/deps/lua/src/lua_struct.c ++++ b/deps/lua/src/lua_struct.c +@@ -296,24 +296,25 @@ static int b_unpack (lua_State *L) { + size_t ld; + const char *data = luaL_checklstring(L, 2, &ld); + size_t pos = luaL_optinteger(L, 3, 1); ++ int n = 0; /* number of results */ + luaL_argcheck(L, pos > 0, 3, "offset must be 1 or greater"); + pos--; /* Lua indexes are 1-based, but here we want 0-based for C + * pointer math. */ + defaultoptions(&h); +- lua_settop(L, 2); + while (*fmt) { + int opt = *fmt++; + size_t size = optsize(L, opt, &fmt); + pos += gettoalign(pos, &h, opt, size); + luaL_argcheck(L, size <= ld && pos <= ld - size, + 2, "data string too short"); +- luaL_checkstack(L, 1, "too many results"); ++ /* stack space for item + next position */ ++ luaL_checkstack(L, 2, "too many results"); + switch (opt) { + case 'b': case 'B': case 'h': case 'H': + case 'l': case 'L': case 'T': case 'i': case 'I': { /* integer types */ + int issigned = islower(opt); + lua_Number res = getinteger(data+pos, h.endian, issigned, size); +- lua_pushnumber(L, res); ++ lua_pushnumber(L, res); n++; + break; + } + case 'x': { +@@ -323,25 +324,26 @@ static int b_unpack (lua_State *L) { + float f; + memcpy(&f, data+pos, size); + correctbytes((char *)&f, sizeof(f), h.endian); +- lua_pushnumber(L, f); ++ lua_pushnumber(L, f); n++; + break; + } + case 'd': { + double d; + memcpy(&d, data+pos, size); + correctbytes((char *)&d, sizeof(d), h.endian); +- lua_pushnumber(L, d); ++ lua_pushnumber(L, d); n++; + break; + } + case 'c': { + if (size == 0) { +- if (!lua_isnumber(L, -1)) +- luaL_error(L, "format `c0' needs a previous size"); ++ if (n == 0 || !lua_isnumber(L, -1)) ++ luaL_error(L, "format 'c0' needs a previous size"); + size = lua_tonumber(L, -1); +- lua_pop(L, 1); +- luaL_argcheck(L, pos+size <= ld, 2, "data string too short"); ++ lua_pop(L, 1); n--; ++ luaL_argcheck(L, size <= ld && pos <= ld - size, ++ 2, "data string too short"); + } +- lua_pushlstring(L, data+pos, size); ++ lua_pushlstring(L, data+pos, size); n++; + break; + } + case 's': { +@@ -349,15 +351,15 @@ static int b_unpack (lua_State *L) { + if (e == NULL) + luaL_error(L, "unfinished string in data"); + size = (e - (data+pos)) + 1; +- lua_pushlstring(L, data+pos, size - 1); ++ lua_pushlstring(L, data+pos, size - 1); n++; + break; + } + default: controloptions(L, opt, &fmt, &h); + } + pos += size; + } +- lua_pushinteger(L, pos + 1); +- return lua_gettop(L) - 2; ++ lua_pushinteger(L, pos + 1); /* next position */ ++ return n + 1; + } + + diff --git a/debian/patches/0013-Security-fix-Lua-cmsgpack-library-stack-overflow.patch b/debian/patches/0013-Security-fix-Lua-cmsgpack-library-stack-overflow.patch new file mode 100644 index 00000000..141def3b --- /dev/null +++ b/debian/patches/0013-Security-fix-Lua-cmsgpack-library-stack-overflow.patch @@ -0,0 +1,68 @@ +From: antirez <anti...@gmail.com> +Date: Mon, 14 May 2018 17:45:40 +0200 +Subject: Security: fix Lua cmsgpack library stack overflow. + +During an auditing effort, the Apple Vulnerability Research team discovered +a critical Redis security issue affecting the Lua scripting part of Redis. + +-- Description of the problem + +Several years ago I merged a pull request including many small changes at +the Lua MsgPack library (that originally I authored myself). The Pull +Request entered Redis in commit 90b6337c1, in 2014. +Unfortunately one of the changes included a variadic Lua function that +lacked the check for the available Lua C stack. As a result, calling the +"pack" MsgPack library function with a large number of arguments, results +into pushing into the Lua C stack a number of new values proportional to +the number of arguments the function was called with. The pushed values, +moreover, are controlled by untrusted user input. + +This in turn causes stack smashing which we believe to be exploitable, +while not very deterministic, but it is likely that an exploit could be +created targeting specific versions of Redis executables. However at its +minimum the issue results in a DoS, crashing the Redis server. + +-- Versions affected + +Versions greater or equal to Redis 2.8.18 are affected. + +-- Reproducing + +Reproduce with this (based on the original reproduction script by +Apple security team): + +https://gist.github.com/antirez/82445fcbea6d9b19f97014cc6cc79f8a + +-- Verification of the fix + +The fix was tested in the following way: + +1) I checked that the problem is no longer observable running the trigger. +2) The Lua code was analyzed to understand the stack semantics, and that +actually enough stack is allocated in all the cases of mp_pack() calls. +3) The mp_pack() function was modified in order to show exactly what items +in the stack were being set, to make sure that there is no silent overflow +even after the fix. + +-- Credits + +Thank you to the Apple team and to the other persons that helped me +checking the patch and coordinating this communication. +--- + deps/lua/src/lua_cmsgpack.c | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/deps/lua/src/lua_cmsgpack.c b/deps/lua/src/lua_cmsgpack.c +index 1a5749c..8921547 100644 +--- a/deps/lua/src/lua_cmsgpack.c ++++ b/deps/lua/src/lua_cmsgpack.c +@@ -517,6 +517,9 @@ int mp_pack(lua_State *L) { + if (nargs == 0) + return luaL_argerror(L, 0, "MessagePack pack needs input."); + ++ if (!lua_checkstack(L, nargs)) ++ return luaL_argerror(L, 0, "Too many arguments for MessagePack pack."); ++ + buf = mp_buf_new(L); + for(i = 1; i <= nargs; i++) { + /* Copy argument i to top of stack for _encode processing; diff --git a/debian/patches/series b/debian/patches/series index 35c6cec5..83aef545 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -5,3 +5,8 @@ 0005-SOURCE_DATE_EPOCH.patch 0006-Skip-logging-tests-as-not-all-architectures-support-.patch 0008-Drop-tests-with-timing-issues.patch +0009-Security-fix-redis-cli-buffer-overflow.patch +0010-Security-fix-Lua-struct-package-offset-handling.patch +0011-Security-more-cmsgpack-fixes-by-soloestoy.patch +0012-Security-update-Lua-struct-package-for-security.patch +0013-Security-fix-Lua-cmsgpack-library-stack-overflow.patch