fix false success and a memory leak for ACL selector with bad parenthesis combination (#12452)

When doing merge selector, we should check whether the merge
has started (i.e., whether open_bracket_start is -1) every time.
Otherwise, encountering an illegal selector pattern could succeed
and also cause memory leaks, for example:

```
acl setuser test1 (+PING (+SELECT (+DEL )
```

The above would leak memory and succeed with only DEL being applied,
and would now error after the fix.

Co-authored-by: Oran Agra <oran@redislabs.com>
This commit is contained in:
zhaozhao.zz 2023-08-02 15:46:06 +08:00 committed by GitHub
parent b653c759cd
commit 90ab91f00b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 11 additions and 1 deletions

View File

@ -1990,7 +1990,8 @@ sds *ACLMergeSelectorArguments(sds *argv, int argc, int *merged_argc, int *inval
for (int j = 0; j < argc; j++) {
char *op = argv[j];
if (op[0] == '(' && op[sdslen(op) - 1] != ')') {
if (open_bracket_start == -1 &&
(op[0] == '(' && op[sdslen(op) - 1] != ')')) {
selector = sdsdup(argv[j]);
open_bracket_start = j;
continue;

View File

@ -47,6 +47,15 @@ start_server {tags {"acl external:skip"}} {
catch {r ACL SETUSER selector-syntax on (&* &fail)} e
assert_match "*ERR Error in ACL SETUSER modifier '(*)*Adding a pattern after the*" $e
catch {r ACL SETUSER selector-syntax on (+PING (+SELECT (+DEL} e
assert_match "*ERR Unmatched parenthesis in acl selector*" $e
catch {r ACL SETUSER selector-syntax on (+PING (+SELECT (+DEL ) ) ) } e
assert_match "*ERR Error in ACL SETUSER modifier*" $e
catch {r ACL SETUSER selector-syntax on (+PING (+SELECT (+DEL ) } e
assert_match "*ERR Error in ACL SETUSER modifier*" $e
assert_equal "" [r ACL GETUSER selector-syntax]
}