From 503a5a24fb6cf3bd95590a53d14fce82086be52c Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Wed, 15 Apr 2020 16:39:42 +0200
Subject: [PATCH] Don't allow empty spaces in ACL usernames.

Fixes issue #6418.
---
 src/acl.c | 44 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/src/acl.c b/src/acl.c
index a5e35c4d1..75b954c5e 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -170,6 +170,18 @@ sds ACLHashPassword(unsigned char *cleartext, size_t len) {
  * Low level ACL API
  * ==========================================================================*/
 
+/* Return 1 if the specified string contains spaces or null characters.
+ * We do this for usernames and key patterns for simpler rewriting of
+ * ACL rules, presentation on ACL list, and to avoid subtle security bugs
+ * that may arise from parsing the rules in presence of escapes.
+ * The function returns 0 if the string has no spaces. */
+int ACLStringHasSpaces(const char *s, size_t len) {
+    for (size_t i = 0; i < len; i++) {
+        if (isspace(s[i]) || s[i] == 0) return 1;
+    }
+    return 0;
+}
+
 /* Given the category name the command returns the corresponding flag, or
  * zero if there is no match. */
 uint64_t ACLGetCommandCategoryFlagByName(const char *name) {
@@ -791,14 +803,9 @@ int ACLSetUser(user *u, const char *op, ssize_t oplen) {
             errno = EEXIST;
             return C_ERR;
         }
-        /* Validate the pattern: no spaces nor null characters
-         * are allowed, for simpler rewriting of the ACLs without
-         * using quoting. */
-        for (int i = 1; i < oplen; i++) {
-            if (isspace(op[i]) || op[i] == 0) {
-                errno = EINVAL;
-                return C_ERR;
-            }
+        if (ACLStringHasSpaces(op+1,oplen-1)) {
+            errno = EINVAL;
+            return C_ERR;
         }
         sds newpat = sdsnewlen(op+1,oplen-1);
         listNode *ln = listSearchKey(u->patterns,newpat);
@@ -1175,6 +1182,12 @@ int ACLLoadConfiguredUsers(void) {
     while ((ln = listNext(&li)) != NULL) {
         sds *aclrules = listNodeValue(ln);
         sds username = aclrules[0];
+
+        if (ACLStringHasSpaces(aclrules[0],sdslen(aclrules[0]))) {
+            serverLog(LL_WARNING,"Spaces not allowed in ACL usernames");
+            return C_ERR;
+        }
+
         user *u = ACLCreateUser(username,sdslen(username));
         if (!u) {
             u = ACLGetUserByName(username,sdslen(username));
@@ -1300,6 +1313,14 @@ sds ACLLoadFromFile(const char *filename) {
             continue;
         }
 
+        /* Spaces are not allowed in usernames. */
+        if (ACLStringHasSpaces(argv[1],sdslen(argv[1]))) {
+            errors = sdscatprintf(errors,
+                     "'%s:%d: username '%s' contains invalid characters. ",
+                     server.acl_filename, linenum, argv[1]);
+            continue;
+        }
+
         /* Try to process the line using the fake user to validate iif
          * the rules are able to apply cleanly. */
         ACLSetUser(fakeuser,"reset",-1);
@@ -1609,6 +1630,13 @@ void aclCommand(client *c) {
     char *sub = c->argv[1]->ptr;
     if (!strcasecmp(sub,"setuser") && c->argc >= 3) {
         sds username = c->argv[2]->ptr;
+        /* Check username validity. */
+        if (ACLStringHasSpaces(username,sdslen(username))) {
+            addReplyErrorFormat(c,
+                "Usernames can't contain spaces or null characters");
+            return;
+        }
+
         /* Create a temporary user to validate and stage all changes against
          * before applying to an existing user or creating a new user. If all
          * arguments are valid the user parameters will all be applied together.