From c40d23b89fbee79506e73d1e44ed4ba9ea60ecd9 Mon Sep 17 00:00:00 2001 From: Madelyn Olson <34459052+madolson@users.noreply.github.com> Date: Mon, 13 Dec 2021 08:39:04 -0800 Subject: [PATCH] Redact ACL SETUSER arguments if the user has spaces (#9935) --- src/acl.c | 12 ++++++------ tests/unit/slowlog.tcl | 6 ++++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/acl.c b/src/acl.c index b9efd5401..6378d6fde 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1966,6 +1966,12 @@ void addACLLogEntry(client *c, int reason, int context, int argpos, sds username void aclCommand(client *c) { char *sub = c->argv[1]->ptr; if (!strcasecmp(sub,"setuser") && c->argc >= 3) { + /* Initially redact all of the arguments to not leak any information + * about the user. */ + for (int j = 2; j < c->argc; j++) { + redactClientCommandArgument(c, j); + } + sds username = c->argv[2]->ptr; /* Check username validity. */ if (ACLStringHasSpaces(username,sdslen(username))) { @@ -1982,12 +1988,6 @@ void aclCommand(client *c) { user *u = ACLGetUserByName(username,sdslen(username)); if (u) ACLCopyUser(tempu, u); - /* Initially redact all of the arguments to not leak any information - * about the user. */ - for (int j = 2; j < c->argc; j++) { - redactClientCommandArgument(c, j); - } - for (int j = 3; j < c->argc; j++) { if (ACLSetUser(tempu,c->argv[j]->ptr,sdslen(c->argv[j]->ptr)) != C_OK) { const char *errmsg = ACLSetUserStringError(); diff --git a/tests/unit/slowlog.tcl b/tests/unit/slowlog.tcl index b9ac9ddf2..2f4fb35e3 100644 --- a/tests/unit/slowlog.tcl +++ b/tests/unit/slowlog.tcl @@ -49,6 +49,7 @@ start_server {tags {"slowlog"} overrides {slowlog-log-slower-than 1000000}} { test {SLOWLOG - Certain commands are omitted that contain sensitive information} { r config set slowlog-log-slower-than 0 r slowlog reset + catch {r acl setuser "slowlog test user" +get +set} _ r config set masterauth "" r acl setuser slowlog-test-user +get +set r config set slowlog-log-slower-than 0 @@ -57,8 +58,9 @@ start_server {tags {"slowlog"} overrides {slowlog-log-slower-than 1000000}} { # Make sure normal configs work, but the two sensitive # commands are omitted or redacted - assert_equal 4 [llength $slowlog_resp] - assert_equal {slowlog reset} [lindex [lindex [r slowlog get] 3] 3] + assert_equal 5 [llength $slowlog_resp] + assert_equal {slowlog reset} [lindex [lindex [r slowlog get] 4] 3] + assert_equal {acl setuser (redacted) (redacted) (redacted)} [lindex [lindex [r slowlog get] 3] 3] assert_equal {config set masterauth (redacted)} [lindex [lindex [r slowlog get] 2] 3] assert_equal {acl setuser (redacted) (redacted) (redacted)} [lindex [lindex [r slowlog get] 1] 3] assert_equal {config set slowlog-log-slower-than 0} [lindex [lindex [r slowlog get] 0] 3]