sdsrange overflow fix (#830)

Fix overflow bug in `sdsrange`

9 files changed, 30 insertions, 20 deletions

diff --git a/hiredis.c b/hiredis.c
index 3133716..4487134 100644
--- a/hiredis.c
+++ b/hiredis.c
@@ -920,17 +920,17 @@ int redisBufferWrite(redisContext *c, int *done) {
         return REDIS_ERR;
 
     if (sdslen(c->obuf) > 0) {
-        int nwritten = c->funcs->write(c);
+        ssize_t nwritten = c->funcs->write(c);
         if (nwritten < 0) {
             return REDIS_ERR;
         } else if (nwritten > 0) {
-            if (nwritten == (signed)sdslen(c->obuf)) {
+            if (nwritten == (ssize_t)sdslen(c->obuf)) {
                 sdsfree(c->obuf);
                 c->obuf = sdsempty();
                 if (c->obuf == NULL)
                     goto oom;
             } else {
-                sdsrange(c->obuf,nwritten,-1);
+                if (sdsrange(c->obuf,nwritten,-1) < 0) goto oom;
             }
         }
     }
diff --git a/hiredis.h b/hiredis.h
index 9282671..bf10509 100644
--- a/hiredis.h
+++ b/hiredis.h
@@ -39,6 +39,7 @@
 #include <sys/time.h> /* for struct timeval */
 #else
 struct timeval; /* forward declaration */
+typedef long long ssize_t;
 #endif
 #include <stdint.h> /* uintXX_t, etc */
 #include "sds.h" /* for sds */
@@ -200,8 +201,8 @@ typedef struct redisContextFuncs {
     void (*free_privdata)(void *);
     void (*async_read)(struct redisAsyncContext *);
     void (*async_write)(struct redisAsyncContext *);
-    int (*read)(struct redisContext *, char *, size_t);
-    int (*write)(struct redisContext *);
+    ssize_t (*read)(struct redisContext *, char *, size_t);
+    ssize_t (*write)(struct redisContext *);
 } redisContextFuncs;
 
 /* Context for a connection to Redis */
diff --git a/net.c b/net.c
index 54d904d..aa5b183 100644
--- a/net.c
+++ b/net.c
@@ -57,8 +57,8 @@ void redisNetClose(redisContext *c) {
     }
 }
 
-int redisNetRead(redisContext *c, char *buf, size_t bufcap) {
-    int nread = recv(c->fd, buf, bufcap, 0);
+ssize_t redisNetRead(redisContext *c, char *buf, size_t bufcap) {
+    ssize_t nread = recv(c->fd, buf, bufcap, 0);
     if (nread == -1) {
         if ((errno == EWOULDBLOCK && !(c->flags & REDIS_BLOCK)) || (errno == EINTR)) {
             /* Try again later */
@@ -79,8 +79,8 @@ int redisNetRead(redisContext *c, char *buf, size_t bufcap) {
     }
 }
 
-int redisNetWrite(redisContext *c) {
-    int nwritten = send(c->fd, c->obuf, sdslen(c->obuf), 0);
+ssize_t redisNetWrite(redisContext *c) {
+    ssize_t nwritten = send(c->fd, c->obuf, sdslen(c->obuf), 0);
     if (nwritten < 0) {
         if ((errno == EWOULDBLOCK && !(c->flags & REDIS_BLOCK)) || (errno == EINTR)) {
             /* Try again later */
diff --git a/net.h b/net.h
index a4393c0..e28c403 100644
--- a/net.h
+++ b/net.h
@@ -38,8 +38,8 @@
 #include "hiredis.h"
 
 void redisNetClose(redisContext *c);
-int redisNetRead(redisContext *c, char *buf, size_t bufcap);
-int redisNetWrite(redisContext *c);
+ssize_t redisNetRead(redisContext *c, char *buf, size_t bufcap);
+ssize_t redisNetWrite(redisContext *c);
 
 int redisCheckSocketError(redisContext *c);
 int redisContextSetTimeout(redisContext *c, const struct timeval tv);
diff --git a/read.c b/read.c
index d10b62a..544626e 100644
--- a/read.c
+++ b/read.c
@@ -720,7 +720,7 @@ int redisReaderGetReply(redisReader *r, void **reply) {
     /* Discard part of the buffer when we've consumed at least 1k, to avoid
      * doing unnecessary calls to memmove() in sds.c. */
     if (r->pos >= 1024) {
-        sdsrange(r->buf,r->pos,-1);
+        if (sdsrange(r->buf,r->pos,-1) < 0) return REDIS_ERR;
         r->pos = 0;
         r->len = sdslen(r->buf);
     }
diff --git a/sds.c b/sds.c
index f7811a7..49d2096 100644
--- a/sds.c
+++ b/sds.c
@@ -36,6 +36,7 @@
 #include <string.h>
 #include <ctype.h>
 #include <assert.h>
+#include <limits.h>
 #include "sds.h"
 #include "sdsalloc.h"
 
@@ -713,15 +714,20 @@ sds sdstrim(sds s, const char *cset) {
  *
  * The string is modified in-place.
  *
+ * Return value:
+ * -1 (error) if sdslen(s) is larger than maximum positive ssize_t value.
+ *  0 on success.
+ *
  * Example:
  *
  * s = sdsnew("Hello World");
  * sdsrange(s,1,-1); => "ello World"
  */
-void sdsrange(sds s, int start, int end) {
+int sdsrange(sds s, ssize_t start, ssize_t end) {
     size_t newlen, len = sdslen(s);
+    if (len > SSIZE_MAX) return -1;
 
-    if (len == 0) return;
+    if (len == 0) return 0;
     if (start < 0) {
         start = len+start;
         if (start < 0) start = 0;
@@ -732,9 +738,9 @@ void sdsrange(sds s, int start, int end) {
     }
     newlen = (start > end) ? 0 : (end-start)+1;
     if (newlen != 0) {
-        if (start >= (signed)len) {
+        if (start >= (ssize_t)len) {
             newlen = 0;
-        } else if (end >= (signed)len) {
+        } else if (end >= (ssize_t)len) {
             end = len-1;
             newlen = (start > end) ? 0 : (end-start)+1;
         }
@@ -744,6 +750,7 @@ void sdsrange(sds s, int start, int end) {
     if (start && newlen) memmove(s, s+start, newlen);
     s[newlen] = 0;
     sdssetlen(s,newlen);
+    return 0;
 }
 
 /* Apply tolower() to every character of the sds string 's'. */
diff --git a/sds.h b/sds.h
index 3f9a964..eda8833 100644
--- a/sds.h
+++ b/sds.h
@@ -36,6 +36,8 @@
 #define SDS_MAX_PREALLOC (1024*1024)
 #ifdef _MSC_VER
 #define __attribute__(x)
+typedef long long ssize_t;
+#define SSIZE_MAX (LLONG_MAX >> 1)
 #endif
 
 #include <sys/types.h>
@@ -239,7 +241,7 @@ sds sdscatprintf(sds s, const char *fmt, ...);
 
 sds sdscatfmt(sds s, char const *fmt, ...);
 sds sdstrim(sds s, const char *cset);
-void sdsrange(sds s, int start, int end);
+int sdsrange(sds s, ssize_t start, ssize_t end);
 void sdsupdatelen(sds s);
 void sdsclear(sds s);
 int sdscmp(const sds s1, const sds s2);
diff --git a/sockcompat.h b/sockcompat.h
index 56006c1..9249d5e 100644
--- a/sockcompat.h
+++ b/sockcompat.h
@@ -51,7 +51,7 @@
 #include <stddef.h>
 
 #ifdef _MSC_VER
-typedef signed long ssize_t;
+typedef long long ssize_t;
 #endif
 
 /* Emulate the parts of the BSD socket API that we need (override the winsock signatures). */
diff --git a/ssl.c b/ssl.c
index f3870d5..10cf607 100644
--- a/ssl.c
+++ b/ssl.c
@@ -392,7 +392,7 @@ static void redisSSLFree(void *privdata){
     hi_free(rsc);
 }
 
-static int redisSSLRead(redisContext *c, char *buf, size_t bufcap) {
+static ssize_t redisSSLRead(redisContext *c, char *buf, size_t bufcap) {
     redisSSL *rssl = c->privdata;
 
     int nread = SSL_read(rssl->ssl, buf, bufcap);
@@ -434,7 +434,7 @@ static int redisSSLRead(redisContext *c, char *buf, size_t bufcap) {
     }
 }
 
-static int redisSSLWrite(redisContext *c) {
+static ssize_t redisSSLWrite(redisContext *c) {
     redisSSL *rssl = c->privdata;
 
     size_t len = rssl->lastLen ? rssl->lastLen : sdslen(c->obuf);