From 109197585762986502d3a8fa628acc1b82b68cf3 Mon Sep 17 00:00:00 2001 From: Justin Brewer Date: Thu, 17 May 2018 20:17:13 -0500 Subject: 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 --- 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; -- cgit v1.2.3