Fix Read/Write key pattern selector (CVE-2024-51741) (#1514)

The explanation on the original commit was wrong. Key based access must
have a `~` in order to correctly configure whey key prefixes to apply
the selector to. If this is missing, a server assert will be triggered
later.

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: YaacovHazan <yaacov.hazan@redis.com>
This commit is contained in:
Madelyn Olson 2025-01-06 14:02:16 -08:00
parent be25f01e82
commit 08fa278379
2 changed files with 30 additions and 4 deletions

View File

@ -1074,19 +1074,24 @@ int ACLSetSelector(aclSelector *selector, const char *op, size_t oplen) {
int flags = 0; int flags = 0;
size_t offset = 1; size_t offset = 1;
if (op[0] == '%') { if (op[0] == '%') {
int perm_ok = 1;
for (; offset < oplen; offset++) { for (; offset < oplen; offset++) {
if (toupper(op[offset]) == 'R' && !(flags & ACL_READ_PERMISSION)) { if (toupper(op[offset]) == 'R' && !(flags & ACL_READ_PERMISSION)) {
flags |= ACL_READ_PERMISSION; flags |= ACL_READ_PERMISSION;
} else if (toupper(op[offset]) == 'W' && !(flags & ACL_WRITE_PERMISSION)) { } else if (toupper(op[offset]) == 'W' && !(flags & ACL_WRITE_PERMISSION)) {
flags |= ACL_WRITE_PERMISSION; flags |= ACL_WRITE_PERMISSION;
} else if (op[offset] == '~' && flags) { } else if (op[offset] == '~') {
offset++; offset++;
break; break;
} else { } else {
errno = EINVAL; perm_ok = 0;
return C_ERR; break;
} }
} }
if (!flags || !perm_ok) {
errno = EINVAL;
return C_ERR;
}
} else { } else {
flags = ACL_ALL_PERMISSION; flags = ACL_ALL_PERMISSION;
} }

View File

@ -116,11 +116,32 @@ start_server {tags {"acl external:skip"}} {
assert_match "*NOPERM*key*" $err assert_match "*NOPERM*key*" $err
} }
test {Validate read and write permissions format} { test {Validate read and write permissions format - empty permission} {
catch {r ACL SETUSER key-permission-RW %~} err catch {r ACL SETUSER key-permission-RW %~} err
set err set err
} {ERR Error in ACL SETUSER modifier '%~': Syntax error} } {ERR Error in ACL SETUSER modifier '%~': Syntax error}
test {Validate read and write permissions format - empty selector} {
catch {r ACL SETUSER key-permission-RW %} err
set err
} {ERR Error in ACL SETUSER modifier '%': Syntax error}
test {Validate read and write permissions format - empty pattern} {
# Empty pattern results with R/W access to no key
r ACL SETUSER key-permission-RW on nopass %RW~ +@all
$r2 auth key-permission-RW password
catch {$r2 SET x 5} err
set err
} {NOPERM No permissions to access a key}
test {Validate read and write permissions format - no pattern} {
# No pattern results with R/W access to no key (currently we accept this syntax error)
r ACL SETUSER key-permission-RW on nopass %RW +@all
$r2 auth key-permission-RW password
catch {$r2 SET x 5} err
set err
} {NOPERM No permissions to access a key}
test {Test separate read and write permissions on different selectors are not additive} { test {Test separate read and write permissions on different selectors are not additive} {
r ACL SETUSER key-permission-RW-selector on nopass "(%R~read* +@all)" "(%W~write* +@all)" r ACL SETUSER key-permission-RW-selector on nopass "(%R~read* +@all)" "(%W~write* +@all)"
$r2 auth key-permission-RW-selector password $r2 auth key-permission-RW-selector password