Merge pull request redis#6812 from guybe7/str_convert_fix

ld2string should fail if string contains \0 in the middle
oranagra · Jan 30, 2020 · 53ac8c7 · 53ac8c7
2 parents 17ff317 + 2deb555
commit 53ac8c7
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 11 deletions.
diff --git a/src/module.c b/src/module.c
@@ -3901,7 +3901,7 @@ void RM_SaveLongDouble(RedisModuleIO *io, long double value) {
     /* Long double has different number of bits in different platforms, so we
      * save it as a string type. */
     size_t len = ld2string(buf,sizeof(buf),value,LD_STR_HEX);
-    RM_SaveStringBuffer(io,buf,len+1); /* len+1 for '\0' */
+    RM_SaveStringBuffer(io,buf,len);
 }
 
 /* In the context of the rdb_save method of a module data type, loads back the

diff --git a/src/object.c b/src/object.c
@@ -640,21 +640,13 @@ int getDoubleFromObjectOrReply(client *c, robj *o, double *target, const char *m
 
 int getLongDoubleFromObject(robj *o, long double *target) {
     long double value;
-    char *eptr;
 
     if (o == NULL) {
         value = 0;
     } else {
         serverAssertWithInfo(NULL,o,o->type == OBJ_STRING);
         if (sdsEncodedObject(o)) {
-            errno = 0;
-            value = strtold(o->ptr, &eptr);
-            if (sdslen(o->ptr) == 0 ||
-                isspace(((const char*)o->ptr)[0]) ||
-                (size_t)(eptr-(char*)o->ptr) != sdslen(o->ptr) ||
-                (errno == ERANGE &&
-                    (value == HUGE_VAL || value == -HUGE_VAL || value == 0)) ||
-                isnan(value))
+            if (!string2ld(o->ptr, sdslen(o->ptr), &value))
                 return C_ERR;
         } else if (o->encoding == OBJ_ENCODING_INT) {
             value = (long)o->ptr;

diff --git a/src/util.c b/src/util.c
@@ -471,13 +471,14 @@ int string2ld(const char *s, size_t slen, long double *dp) {
     long double value;
     char *eptr;
 
-    if (slen >= sizeof(buf)) return 0;
+    if (slen == 0 || slen >= sizeof(buf)) return 0;
     memcpy(buf,s,slen);
     buf[slen] = '\0';
 
     errno = 0;
     value = strtold(buf, &eptr);
     if (isspace(buf[0]) || eptr[0] != '\0' ||
+        (size_t)(eptr-buf) != slen ||
         (errno == ERANGE &&
             (value == HUGE_VAL || value == -HUGE_VAL || value == 0)) ||
         errno == EINVAL ||

diff --git a/tests/modules/misc.c b/tests/modules/misc.c
@@ -74,6 +74,15 @@ int test_ld_conv(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
         RedisModule_ReplyWithError(ctx, err);
         goto final;
     }
+    /* Make sure we can't convert a string that has \0 in it */
+    char buf[4] = "123";
+    buf[1] = '\0';
+    RedisModuleString *s3 = RedisModule_CreateString(ctx, buf, 3);
+    long double ld3;
+    if (RedisModule_StringToLongDouble(s3, &ld3) == REDISMODULE_OK) {
+        RedisModule_ReplyWithError(ctx, "Invalid string successfully converted to long double");
+        goto final;
+    }
     RedisModule_ReplyWithLongDouble(ctx, ld2);
 final:
     RedisModule_FreeString(ctx, s1);

diff --git a/tests/unit/type/hash.tcl b/tests/unit/type/hash.tcl
@@ -390,6 +390,13 @@ start_server {tags {"hash"}} {
         lappend rv [string match "ERR*not*float*" $bigerr]
     } {1 1}
 
+    test {HINCRBYFLOAT fails against hash value that contains a null-terminator in the middle} {
+        r hset h f "1\x002"
+        catch {r hincrbyfloat h f 1} err
+        set rv {}
+        lappend rv [string match "ERR*not*float*" $err]
+    } {1}
+
     test {HSTRLEN against the small hash} {
         set err {}
         foreach k [array names smallhash *] {