From 109197585762986502d3a8fa628acc1b82b68cf3 Mon Sep 17 00:00:00 2001
From: Justin Brewer <jzb0012@auburn.edu>
Date: Thu, 17 May 2018 20:17:13 -0500
Subject: [PATCH] Fix bulk and multi-bulk length truncation

processMultiBulkItem truncates the value read from readLongLong,
resulting in a corrupted state when the next item is read.
createArray takes an int, so bound to INT_MAX.

Inspection showed that processBulkItem had the same truncation issue.
createString takes size_t, so bound to SIZE_MAX. This only has an
effect on 32-bit platforms.

A strict lower bound is also added, since negative lengths other
than -1 are invalid according to RESP.

Signed-off-by: Justin Brewer <jzb0012@auburn.edu>
---
 read.c | 14 +++++++++++++-
 test.c | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/read.c b/read.c
index c0f8489..39c21b8 100644
--- a/read.c
+++ b/read.c
@@ -264,7 +264,13 @@ static int processBulkItem(redisReader *r) {
             return REDIS_ERR;
         }
 
-        if (len < 0) {
+        if (len < -1 || (LLONG_MAX > SIZE_MAX && len > (long long)SIZE_MAX)) {
+            __redisReaderSetError(r,REDIS_ERR_PROTOCOL,
+                    "Bulk string length out of range");
+            return REDIS_ERR;
+        }
+
+        if (len == -1) {
             /* The nil object can always be created. */
             if (r->fn && r->fn->createNil)
                 obj = r->fn->createNil(cur);
@@ -325,6 +331,12 @@ static int processMultiBulkItem(redisReader *r) {
 
         root = (r->ridx == 0);
 
+        if(elements < -1 || elements > INT_MAX) {
+            __redisReaderSetError(r,REDIS_ERR_PROTOCOL,
+                    "Multi-bulk length out of range");
+            return REDIS_ERR;
+        }
+
         if (elements == -1) {
             if (r->fn && r->fn->createNil)
                 obj = r->fn->createNil(cur);
diff --git a/test.c b/test.c
index 844aa5f..4f2dafd 100644
--- a/test.c
+++ b/test.c
@@ -340,6 +340,44 @@ static void test_reply_reader(void) {
     freeReplyObject(reply);
     redisReaderFree(reader);
 
+    test("Set error when array < -1: ");
+    reader = redisReaderCreate();
+    redisReaderFeed(reader, "*-2\r\n+asdf\r\n",12);
+    ret = redisReaderGetReply(reader,&reply);
+    test_cond(ret == REDIS_ERR &&
+              strcasecmp(reader->errstr,"Multi-bulk length out of range") == 0);
+    freeReplyObject(reply);
+    redisReaderFree(reader);
+
+    test("Set error when bulk < -1: ");
+    reader = redisReaderCreate();
+    redisReaderFeed(reader, "$-2\r\nasdf\r\n",11);
+    ret = redisReaderGetReply(reader,&reply);
+    test_cond(ret == REDIS_ERR &&
+              strcasecmp(reader->errstr,"Bulk string length out of range") == 0);
+    freeReplyObject(reply);
+    redisReaderFree(reader);
+
+    test("Set error when array > INT_MAX: ");
+    reader = redisReaderCreate();
+    redisReaderFeed(reader, "*9223372036854775807\r\n+asdf\r\n",29);
+    ret = redisReaderGetReply(reader,&reply);
+    test_cond(ret == REDIS_ERR &&
+            strcasecmp(reader->errstr,"Multi-bulk length out of range") == 0);
+    freeReplyObject(reply);
+    redisReaderFree(reader);
+
+#if LLONG_MAX > SIZE_MAX
+    test("Set error when bulk > SIZE_MAX: ");
+    reader = redisReaderCreate();
+    redisReaderFeed(reader, "$9223372036854775807\r\nasdf\r\n",28);
+    ret = redisReaderGetReply(reader,&reply);
+    test_cond(ret == REDIS_ERR &&
+            strcasecmp(reader->errstr,"Bulk string length out of range") == 0);
+    freeReplyObject(reply);
+    redisReaderFree(reader);
+#endif
+
     test("Works with NULL functions for reply: ");
     reader = redisReaderCreate();
     reader->fn = NULL;