1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
|
From 4bb7ae751d8a0b2b277b395a3b9b004197d0cb69 Mon Sep 17 00:00:00 2001
From: Madelyn Olson <madelyneolson@gmail.com>
Date: Mon, 6 Jan 2025 14:02:16 -0800
Subject: 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>
---
src/acl.c | 11 ++++++++---
tests/unit/acl-v2.tcl | 23 ++++++++++++++++++++++-
2 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/src/acl.c b/src/acl.c
index 6b53d901c..ed6dc97e4 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -1031,19 +1031,24 @@ int ACLSetSelector(aclSelector *selector, const char* op, size_t oplen) {
int flags = 0;
size_t offset = 1;
if (op[0] == '%') {
+ int perm_ok = 1;
for (; offset < oplen; offset++) {
if (toupper(op[offset]) == 'R' && !(flags & ACL_READ_PERMISSION)) {
flags |= ACL_READ_PERMISSION;
} else if (toupper(op[offset]) == 'W' && !(flags & ACL_WRITE_PERMISSION)) {
flags |= ACL_WRITE_PERMISSION;
- } else if (op[offset] == '~' && flags) {
+ } else if (op[offset] == '~') {
offset++;
break;
} else {
- errno = EINVAL;
- return C_ERR;
+ perm_ok = 0;
+ break;
}
}
+ if (!flags || !perm_ok) {
+ errno = EINVAL;
+ return C_ERR;
+ }
} else {
flags = ACL_ALL_PERMISSION;
}
diff --git a/tests/unit/acl-v2.tcl b/tests/unit/acl-v2.tcl
index 114fadec3..847980c8a 100644
--- a/tests/unit/acl-v2.tcl
+++ b/tests/unit/acl-v2.tcl
@@ -107,11 +107,32 @@ start_server {tags {"acl external:skip"}} {
assert_match "*NOPERM*keys*" $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
set err
} {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 this user has no permissions to access one of the keys used as arguments}
+
+ 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 this user has no permissions to access one of the keys used as arguments}
+
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)"
$r2 auth key-permission-RW-selector password
--
2.30.2
|
