Escape unsafe field name characters in INFO. (#8492)

Fixes #8489

src/module.c

@@ -6759,10 +6759,13 @@ int RM_InfoBeginDictField(RedisModuleInfoCtx *ctx, char *name) {
     /* Implicitly end dicts, instead of returning an error which is likely un checked. */
     if (ctx->in_dict_field)
         RM_InfoEndDictField(ctx);
+    char *tmpmodname, *tmpname;
     ctx->info = sdscatfmt(ctx->info,
         "%s_%s:",
-        ctx->module->name,
-        name);
+        getSafeInfoString(ctx->module->name, strlen(ctx->module->name), &tmpmodname),
+        getSafeInfoString(name, strlen(name), &tmpname));
+    if (tmpmodname != NULL) zfree(tmpmodname);
+    if (tmpname != NULL) zfree(tmpname);
     ctx->in_dict_field = 1;
     return REDISMODULE_OK;
 }

src/server.c

@@ -4501,6 +4501,25 @@ void bytesToHuman(char *s, unsigned long long n) {
     }
 }
 
+/* Characters we sanitize on INFO output to maintain expected format. */
+static char unsafe_info_chars[] = "#:\n\r";
+static char unsafe_info_chars_substs[] = "____";   /* Must be same length as above */
+
+/* Returns a sanitized version of s that contains no unsafe info string chars.
+ * If no unsafe characters are found, simply returns s. Caller needs to
+ * free tmp if it is non-null on return.
+ */
+const char *getSafeInfoString(const char *s, size_t len, char **tmp) {
+    *tmp = NULL;
+    if (mempbrk(s, len, unsafe_info_chars,sizeof(unsafe_info_chars)-1)
+        == NULL) return s;
+    char *new = *tmp = zmalloc(len + 1);
+    memcpy(new, s, len);
+    new[len] = '\0';
+    return memmapchars(new, len, unsafe_info_chars, unsafe_info_chars_substs,
+                       sizeof(unsafe_info_chars)-1);
+}
+
 /* Create the string returned by the INFO command. This is decoupled
  * by the INFO command itself as we need to report the same information
  * on memory corruption problems. */
@@ -5126,15 +5145,17 @@ sds genRedisInfoString(const char *section) {
         dictIterator *di;
         di = dictGetSafeIterator(server.commands);
         while((de = dictNext(di)) != NULL) {
+            char *tmpsafe;
             c = (struct redisCommand *) dictGetVal(de);
             if (!c->calls && !c->failed_calls && !c->rejected_calls)
                 continue;
             info = sdscatprintf(info,
                 "cmdstat_%s:calls=%lld,usec=%lld,usec_per_call=%.2f"
                 ",rejected_calls=%lld,failed_calls=%lld\r\n",
-                c->name, c->calls, c->microseconds,
+                getSafeInfoString(c->name, strlen(c->name), &tmpsafe), c->calls, c->microseconds,
                 (c->calls == 0) ? 0 : ((float)c->microseconds/c->calls),
                 c->rejected_calls, c->failed_calls);
+            if (tmpsafe != NULL) zfree(tmpsafe);
         }
         dictReleaseIterator(di);
     }
@@ -5147,10 +5168,12 @@ sds genRedisInfoString(const char *section) {
         raxSeek(&ri,"^",NULL,0);
         struct redisError *e;
         while(raxNext(&ri)) {
+            char *tmpsafe;
             e = (struct redisError *) ri.data;
             info = sdscatprintf(info,
                 "errorstat_%.*s:count=%lld\r\n",
-                (int)ri.key_len, ri.key, e->count);
+                (int)ri.key_len, getSafeInfoString((char *) ri.key, ri.key_len, &tmpsafe), e->count);
+            if (tmpsafe != NULL) zfree(tmpsafe);
         }
         raxStop(&ri);
     }

src/server.h

@@ -2685,6 +2685,7 @@ void _serverPanic(const char *file, int line, const char *msg, ...);
 #endif
 void serverLogObjectDebugInfo(const robj *o);
 void sigsegvHandler(int sig, siginfo_t *info, void *secret);
+const char *getSafeInfoString(const char *s, size_t len, char **tmp);
 sds genRedisInfoString(const char *section);
 sds genModulesInfoString(sds info);
 void enableWatchdog(int period);

src/util.c

@@ -244,6 +244,33 @@ long long memtoll(const char *p, int *err) {
     return val*mul;
 }
 
+/* Search a memory buffer for any set of bytes, like strpbrk().
+ * Returns pointer to first found char or NULL.
+ */
+const char *mempbrk(const char *s, size_t len, const char *chars, size_t charslen) {
+    for (size_t j = 0; j < len; j++) {
+        for (size_t n = 0; n < charslen; n++)
+            if (s[j] == chars[n]) return &s[j];
+    }
+
+    return NULL;
+}
+
+/* Modify the buffer replacing all occurrences of chars from the 'from'
+ * set with the corresponding char in the 'to' set. Always returns s.
+ */
+char *memmapchars(char *s, size_t len, const char *from, const char *to, size_t setlen) {
+    for (size_t j = 0; j < len; j++) {
+        for (size_t i = 0; i < setlen; i++) {
+            if (s[j] == from[i]) {
+                s[j] = to[i];
+                break;
+            }
+        }
+    }
+    return s;
+}
+
 /* Return the number of digits of 'v' when converted to string in radix 10.
  * See ll2string() for more information. */
 uint32_t digits10(uint64_t v) {

src/util.h

@@ -49,6 +49,8 @@ int stringmatchlen(const char *p, int plen, const char *s, int slen, int nocase)
 int stringmatch(const char *p, const char *s, int nocase);
 int stringmatchlen_fuzz_test(void);
 long long memtoll(const char *p, int *err);
+const char *mempbrk(const char *s, size_t len, const char *chars, size_t charslen);
+char *memmapchars(char *s, size_t len, const char *from, const char *to, size_t setlen);
 uint32_t digits10(uint64_t v);
 uint32_t sdigits10(int64_t v);
 int ll2string(char *s, size_t len, long long value);

tests/integration/redis-cli.tcl

@@ -109,7 +109,7 @@ start_server {tags {"cli"}} {
     test_interactive_cli "INFO response should be printed raw" {
         set lines [split [run_command $fd info] "\n"]
         foreach line $lines {
-            assert [regexp {^$|^#|^[a-z0-9_]+:.+} $line]
+            assert [regexp {^$|^#|^[^#:]+:} $line]
         }
     }
 

tests/modules/infotest.c

@@ -21,6 +21,11 @@ void InfoFunc(RedisModuleInfoCtx *ctx, int for_crash_report) {
     RedisModule_InfoAddFieldLongLong(ctx, "expires", 1);
     RedisModule_InfoEndDictField(ctx);
 
+    RedisModule_InfoAddSection(ctx, "unsafe");
+    RedisModule_InfoBeginDictField(ctx, "unsafe:field");
+    RedisModule_InfoAddFieldLongLong(ctx, "value", 1);
+    RedisModule_InfoEndDictField(ctx);
+
     if (for_crash_report) {
         RedisModule_InfoAddSection(ctx, "Klingon");
         RedisModule_InfoAddFieldCString(ctx, "one", "wa’");

tests/unit/info.tcl

@@ -150,4 +150,12 @@ start_server {tags {"info"}} {
             assert_match {} [errorstat NOPERM]
         }
     }
+
+    start_server {} {
+        test {Unsafe command names are sanitized in INFO output} {
+            catch {r host:} e
+            set info [r info commandstats]
+            assert_match {*cmdstat_host_:calls=1*} $info
+        }
+    }
 }

tests/unit/moduleapi/infotest.tcl

@@ -85,5 +85,10 @@ start_server {tags {"modules"}} {
         set keys [scan [regexp -inline {keys\=([\d]*)} $keyspace] keys=%d]
     } {3}
 
+    test {module info unsafe fields} {
+        set info [r info infotest_unsafe]
+        assert_match {*infotest_unsafe_field:value=1*} $info
+    }
+
     # TODO: test crash report.
 }