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 <[email protected]> Thu, 14 Jun 2018 15:08:27 +0200
Regards,
--
,''`.
: :' : Chris Lamb
`. `'` [email protected] / 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 <[email protected]> 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 <[email protected]>
+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 <[email protected]>
+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 <[email protected]>
+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 <[email protected]>
+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 <[email protected]>
+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
