Fix AUTH failure on URIs with empty username in redis-cli (#15255)

Partial fix #11085

## Bug

`parseRedisUri()` called `percentDecode(curr, 0)` for URIs of the form
`redis://:password@host` and stored the resulting **empty-but-non-NULL**
sds into `connInfo->user`.
`cliAuth()` only checks `user == NULL` when deciding between legacy
`AUTH <pass>` and ACL `AUTH <user> <pass>`, so the empty username was
sent as `AUTH "" <password>`, which the server rejects with `WRONGPASS`.

## Fix

In parseRedisUri(), treat explicitly empty username or password
components as NULL rather than empty SDS strings. This allows cliAuth()
to fall back to legacy single-argument AUTH when the username is empty,
and to skip AUTH entirely when the password is empty.

Before replacing URI-parsed credentials, existing connInfo->user and
connInfo->auth values are freed to avoid leaks and preserve the expected
"later arguments override earlier ones" behavior.

As a related cleanup, -a/--pass and --user now also free previously
assigned values before reassignment, fixing the same leak pattern when
those options are specified multiple times.
This commit is contained in:
nuyb 2026-06-02 10:29:56 +09:00 committed by GitHub
parent 230c651c89
commit 09b5c95b98
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 105 additions and 3 deletions

View file

@ -324,11 +324,18 @@ void parseRedisUri(const char *uri, const char* tool_name, cliConnInfo *connInfo
/* Extract user info. */
if ((userinfo = strchr(curr,'@'))) {
if ((username = strchr(curr, ':')) && username < userinfo) {
connInfo->user = percentDecode(curr, username - curr);
/* Free any value previously set via --user / -a (later
* parameters override earlier ones) and use NULL for an
* explicitly empty component, so cliAuth() falls back to the
* legacy single-argument AUTH (empty username) or skips AUTH
* entirely (empty password) instead of sending an empty ACL
* component, which the server rejects. */
sdsfree(connInfo->user);
connInfo->user = (username > curr) ? percentDecode(curr, username - curr) : NULL;
curr = username + 1;
}
connInfo->auth = percentDecode(curr, userinfo - curr);
sdsfree(connInfo->auth);
connInfo->auth = (userinfo > curr) ? percentDecode(curr, userinfo - curr) : NULL;
curr = userinfo + 1;
}
if (curr == end) return;

View file

@ -2735,8 +2735,10 @@ static int parseOptions(int argc, char **argv) {
} else if ((!strcmp(argv[i],"-a") || !strcmp(argv[i],"--pass"))
&& !lastarg)
{
sdsfree(config.conn_info.auth);
config.conn_info.auth = sdsnew(argv[++i]);
} else if (!strcmp(argv[i],"--user") && !lastarg) {
sdsfree(config.conn_info.user);
config.conn_info.user = sdsnew(argv[++i]);
} else if (!strcmp(argv[i],"-u") && !lastarg) {
parseRedisUri(argv[++i],"redis-cli",&config.conn_info,&config.tls);

View file

@ -859,4 +859,97 @@ start_server {tags {"cli external:skip"}} {
}
}
start_server {tags {"cli external:skip"}} {
# Regression for the parseRedisUri() bug where "redis://:password@host"
# produced an empty ACL username and the server replied WRONGPASS.
r config set requirepass "uri-no-username-pass"
set host [srv host]
set port [srv port]
if {$::tls} {
set scheme "rediss"
} else {
set scheme "redis"
}
set tls_args [rediscli_tls_config "tests"]
test "redis-cli -u with empty username falls back to legacy AUTH" {
set cmd [list src/redis-cli --no-auth-warning {*}$tls_args \
-u "$scheme://:uri-no-username-pass@$host:$port" PING]
assert_equal "PONG" [exec {*}$cmd]
}
test "redis-cli -u with explicit username uses ACL AUTH" {
set cmd [list src/redis-cli --no-auth-warning {*}$tls_args \
-u "$scheme://default:uri-no-username-pass@$host:$port" PING]
assert_equal "PONG" [exec {*}$cmd]
}
test "redis-cli -u after -a overrides auth without leaking" {
# -a sets connInfo->auth first, then -u must free the previous
# value before assigning the URI-provided one.
set cmd [list src/redis-cli --no-auth-warning {*}$tls_args \
-a wrong-pass \
-u "$scheme://default:uri-no-username-pass@$host:$port" PING]
assert_equal "PONG" [exec {*}$cmd]
}
test "redis-cli -u after --user overrides user without leaking" {
set cmd [list src/redis-cli --no-auth-warning {*}$tls_args \
--user wronguser \
-u "$scheme://default:uri-no-username-pass@$host:$port" PING]
assert_equal "PONG" [exec {*}$cmd]
}
test "redis-cli -u with explicit empty username clears stale --user" {
# A previously set --user must be cleared (not left stale) when
# the URI explicitly empties the username component, otherwise
# cliAuth() would send ACL AUTH with the stale username instead
# of the intended legacy single-argument AUTH.
set cmd [list src/redis-cli --no-auth-warning {*}$tls_args \
--user wronguser \
-u "$scheme://:uri-no-username-pass@$host:$port" PING]
assert_equal "PONG" [exec {*}$cmd]
}
test "redis-cli -u with empty userinfo overrides previously set -a" {
# "redis://@host" has empty userinfo. Since later parameters
# override earlier ones, the URI clears the password supplied via
# -a, so cliAuth() sends no AUTH and the password-protected server
# rejects the command with NOAUTH.
set cmd [list src/redis-cli --no-auth-warning {*}$tls_args \
-a uri-no-username-pass \
-u "$scheme://@$host:$port" PING]
catch {exec {*}$cmd 2>@1} e
assert_match "*NOAUTH*" $e
}
r config set requirepass ""
}
start_server {tags {"cli external:skip"}} {
# Regression for empty userinfo URIs: "redis://@host" and
# "redis://user:@host" should leave connInfo->auth unset so that
# cliAuth() skips AUTH entirely instead of sending an empty password.
set host [srv host]
set port [srv port]
if {$::tls} {
set scheme "rediss"
} else {
set scheme "redis"
}
set tls_args [rediscli_tls_config "tests"]
test "redis-cli -u with empty userinfo skips AUTH" {
set cmd [list src/redis-cli {*}$tls_args \
-u "$scheme://@$host:$port" PING]
assert_equal "PONG" [exec {*}$cmd]
}
test "redis-cli -u with empty password skips AUTH" {
set cmd [list src/redis-cli {*}$tls_args \
-u "$scheme://default:@$host:$port" PING]
assert_equal "PONG" [exec {*}$cmd]
}
}
file delete ./.rediscli_history_test