zeekling/hiredis
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix bulk and multi-bulk length truncation

Browse Source 
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>
This commit is contained in:
Justin Brewer 2018-05-17 20:17:13 -05:00 committed by michael-grunder
parent 93421f9d84
commit 1091975857
2 changed files with 51 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
read.c
View File

@ -264,7 +264,13 @@ static int processBulkItem(redisReader *r) {
return REDIS_ERR; 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. */ /* The nil object can always be created. */
if (r->fn && r->fn->createNil) if (r->fn && r->fn->createNil)
obj = r->fn->createNil(cur); obj = r->fn->createNil(cur);
@ -325,6 +331,12 @@ static int processMultiBulkItem(redisReader *r) {
root = (r->ridx == 0); 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 (elements == -1) {
if (r->fn && r->fn->createNil) if (r->fn && r->fn->createNil)
obj = r->fn->createNil(cur); obj = r->fn->createNil(cur);

38
test.c
View File

@ -340,6 +340,44 @@ static void test_reply_reader(void) {
freeReplyObject(reply); freeReplyObject(reply);
redisReaderFree(reader); 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: "); test("Works with NULL functions for reply: ");
reader = redisReaderCreate(); reader = redisReaderCreate();
reader->fn = NULL; reader->fn = NULL;