Properly detect integer parse errors

Badly formatted or out-of-range integers are now protocol errors.

Signed-off-by: Justin Brewer <jzb0012@auburn.edu>
This commit is contained in:
Justin Brewer 2018-05-17 18:29:31 -05:00 committed by michael-grunder
parent dbde4f68cf
commit 93421f9d84
2 changed files with 76 additions and 28 deletions

66
read.c
View File

@ -39,6 +39,7 @@
#include <assert.h> #include <assert.h>
#include <errno.h> #include <errno.h>
#include <ctype.h> #include <ctype.h>
#include <limits.h>
#include "read.h" #include "read.h"
#include "sds.h" #include "sds.h"
@ -142,32 +143,24 @@ static char *seekNewline(char *s, size_t len) {
} }
/* Read a long long value starting at *s, under the assumption that it will be /* Read a long long value starting at *s, under the assumption that it will be
* terminated by \r\n. Ambiguously returns -1 for unexpected input. */ * terminated by \r\n. Returns REDIS_ERR for unexpected inputs or if the
static long long readLongLong(char *s) { * resulting value would be greater than LLONG_MAX. */
long long v = 0; static int readLongLong(char *s, long long* val) {
int dec, mult = 1; char* end;
char c; errno = 0;
if (*s == '-') { long long v = strtoll(s, &end, 10);
mult = -1;
s++; if(s == end || ((v == LLONG_MAX || v == LLONG_MIN) && errno == ERANGE)
} else if (*s == '+') { || (*end != '\r' || *(end+1) != '\n')) {
mult = 1; return REDIS_ERR;
s++;
} }
while ((c = *(s++)) != '\r') { if(val) {
dec = c - '0'; *val = v;
if (dec >= 0 && dec < 10) {
v *= 10;
v += dec;
} else {
/* Should not happen... */
return -1;
}
} }
return mult*v; return REDIS_OK;
} }
static char *readLine(redisReader *r, int *_len) { static char *readLine(redisReader *r, int *_len) {
@ -218,10 +211,17 @@ static int processLineItem(redisReader *r) {
if ((p = readLine(r,&len)) != NULL) { if ((p = readLine(r,&len)) != NULL) {
if (cur->type == REDIS_REPLY_INTEGER) { if (cur->type == REDIS_REPLY_INTEGER) {
if (r->fn && r->fn->createInteger) if (r->fn && r->fn->createInteger) {
obj = r->fn->createInteger(cur,readLongLong(p)); long long v;
else if(readLongLong(p, &v) == REDIS_ERR) {
__redisReaderSetError(r,REDIS_ERR_PROTOCOL,
"Bad integer value");
return REDIS_ERR;
}
obj = r->fn->createInteger(cur,v);
} else {
obj = (void*)REDIS_REPLY_INTEGER; obj = (void*)REDIS_REPLY_INTEGER;
}
} else { } else {
/* Type will be error or status. */ /* Type will be error or status. */
if (r->fn && r->fn->createString) if (r->fn && r->fn->createString)
@ -248,7 +248,7 @@ static int processBulkItem(redisReader *r) {
redisReadTask *cur = &(r->rstack[r->ridx]); redisReadTask *cur = &(r->rstack[r->ridx]);
void *obj = NULL; void *obj = NULL;
char *p, *s; char *p, *s;
long len; long long len;
unsigned long bytelen; unsigned long bytelen;
int success = 0; int success = 0;
@ -257,7 +257,12 @@ static int processBulkItem(redisReader *r) {
if (s != NULL) { if (s != NULL) {
p = r->buf+r->pos; p = r->buf+r->pos;
bytelen = s-(r->buf+r->pos)+2; /* include \r\n */ bytelen = s-(r->buf+r->pos)+2; /* include \r\n */
len = readLongLong(p);
if (readLongLong(p, &len) == REDIS_ERR) {
__redisReaderSetError(r,REDIS_ERR_PROTOCOL,
"Bad bulk string length");
return REDIS_ERR;
}
if (len < 0) { if (len < 0) {
/* The nil object can always be created. */ /* The nil object can always be created. */
@ -301,7 +306,7 @@ static int processMultiBulkItem(redisReader *r) {
redisReadTask *cur = &(r->rstack[r->ridx]); redisReadTask *cur = &(r->rstack[r->ridx]);
void *obj; void *obj;
char *p; char *p;
long elements; long long elements;
int root = 0; int root = 0;
/* Set error for nested multi bulks with depth > 7 */ /* Set error for nested multi bulks with depth > 7 */
@ -312,7 +317,12 @@ static int processMultiBulkItem(redisReader *r) {
} }
if ((p = readLine(r,NULL)) != NULL) { if ((p = readLine(r,NULL)) != NULL) {
elements = readLongLong(p); if(readLongLong(p, &elements) == REDIS_ERR) {
__redisReaderSetError(r,REDIS_ERR_PROTOCOL,
"Bad multi-bulk length");
return REDIS_ERR;
}
root = (r->ridx == 0); root = (r->ridx == 0);
if (elements == -1) { if (elements == -1) {

38
test.c
View File

@ -302,6 +302,44 @@ static void test_reply_reader(void) {
strncasecmp(reader->errstr,"No support for",14) == 0); strncasecmp(reader->errstr,"No support for",14) == 0);
redisReaderFree(reader); redisReaderFree(reader);
test("Correctly parses LLONG_MAX: ");
reader = redisReaderCreate();
redisReaderFeed(reader, ":9223372036854775807\r\n",22);
ret = redisReaderGetReply(reader,&reply);
test_cond(ret == REDIS_OK &&
((redisReply*)reply)->type == REDIS_REPLY_INTEGER &&
((redisReply*)reply)->integer == LLONG_MAX);
freeReplyObject(reply);
redisReaderFree(reader);
test("Set error when > LLONG_MAX: ");
reader = redisReaderCreate();
redisReaderFeed(reader, ":9223372036854775808\r\n",22);
ret = redisReaderGetReply(reader,&reply);
test_cond(ret == REDIS_ERR &&
strcasecmp(reader->errstr,"Bad integer value") == 0);
freeReplyObject(reply);
redisReaderFree(reader);
test("Correctly parses LLONG_MIN: ");
reader = redisReaderCreate();
redisReaderFeed(reader, ":-9223372036854775808\r\n",23);
ret = redisReaderGetReply(reader,&reply);
test_cond(ret == REDIS_OK &&
((redisReply*)reply)->type == REDIS_REPLY_INTEGER &&
((redisReply*)reply)->integer == LLONG_MIN);
freeReplyObject(reply);
redisReaderFree(reader);
test("Set error when < LLONG_MIN: ");
reader = redisReaderCreate();
redisReaderFeed(reader, ":-9223372036854775809\r\n",23);
ret = redisReaderGetReply(reader,&reply);
test_cond(ret == REDIS_ERR &&
strcasecmp(reader->errstr,"Bad integer value") == 0);
freeReplyObject(reply);
redisReaderFree(reader);
test("Works with NULL functions for reply: "); test("Works with NULL functions for reply: ");
reader = redisReaderCreate(); reader = redisReaderCreate();
reader->fn = NULL; reader->fn = NULL;