Memory safety / out-of-bounds access in command argument parsing (KEYNUM paths)

diff --git a/src/db.c b/src/db.c index 6d31e7b17f5..626866221a0 100644 --- a/src/db.c +++ b/src/db.c @@ -3143,10 +3143,11 @@ int getKeysUsingKeySpecs(struct redisCommand *cmd, robj **argv, int argc, int se } else if (spec->find_keys_type == KSPEC_FK_KEYNUM) { step = spec->fk.keynum.keystep; long long numkeys; - if (spec->fk.keynum.keynumidx >= argc) + long keynumidx = first + spec->fk.keynum.keynumidx; + if (keynumidx >= argc || keynumidx < 0) goto invalid_spec; - sds keynum_str = argv[first + spec->fk.keynum.keynumidx]->ptr; + sds keynum_str = argv[keynumidx]->ptr; if (!string2ll(keynum_str,sdslen(keynum_str),&numkeys) || numkeys < 0) { /* Unable to parse the numkeys argument or it was invalid */ goto invalid_spec; @@ -3456,6 +3457,10 @@ int genericGetKeys(int storeKeyOfs, int keyCountOfs, int firstKeyOfs, int keySte int i, num; keyReference *keys; + if (keyCountOfs >= argc) { + result->numkeys = 0; + return 0; + } num = atoi(argv[keyCountOfs]->ptr); /* Sanity check. Don't return any key if the command is going to * reply with syntax error. (no input keys). */ diff --git a/src/module.c b/src/module.c index 1c8d79ab552..dda2022f088 100644 --- a/src/module.c +++ b/src/module.c @@ -10429,6 +10429,7 @@ RedisModuleUser *RM_GetModuleUserFromUserName(RedisModuleString *name) { * REDISMODULE_ERR is returned and errno is set to the following values: * * * ENOENT: Specified command does not exist. + * * EINVAL: Invalid number of arguments for the specified command. * * EACCES: Command cannot be executed, according to ACL rules */ int RM_ACLCheckCommandPermissions(RedisModuleUser *user, RedisModuleString **argv, int argc) { @@ -10441,6 +10442,11 @@ int RM_ACLCheckCommandPermissions(RedisModuleUser *user, RedisModuleString **arg return REDISMODULE_ERR; } + if (!commandCheckArity(cmd, argc, NULL)) { + errno = EINVAL; + return REDISMODULE_ERR; + } + if (ACLCheckAllUserCommandPerm(user->user, cmd, argv, argc, NULL, &keyidxptr) != ACL_OK) { errno = EACCES; return REDISMODULE_ERR; diff --git a/src/script_lua.c b/src/script_lua.c index 7a9472fefb6..24ca1ad5e89 100644 --- a/src/script_lua.c +++ b/src/script_lua.c @@ -1132,6 +1132,9 @@ static int luaRedisAclCheckCmdPermissionsCommand(lua_State *lua) { if ((cmd = lookupCommand(argv, argc)) == NULL) { luaPushError(lua, "Invalid command passed to redis.acl_check_cmd()"); raise_error = 1; + } else if (!commandCheckArity(cmd, argc, NULL)) { + luaPushError(lua, "Wrong number of args for redis.acl_check_cmd()"); + raise_error = 1; } else { int keyidxptr; if (ACLCheckAllUserCommandPerm(rctx->original_client->user, cmd, argv, argc, NULL, &keyidxptr) != ACL_OK) { diff --git a/tests/unit/moduleapi/aclcheck.tcl b/tests/unit/moduleapi/aclcheck.tcl index aa571b7413d..aad86b08442 100644 --- a/tests/unit/moduleapi/aclcheck.tcl +++ b/tests/unit/moduleapi/aclcheck.tcl @@ -17,6 +17,11 @@ start_server {tags {"modules acl external:skip"}} { assert {[dict get $entry context] eq {module}} assert {[dict get $entry object] eq {set}} assert {[dict get $entry reason] eq {command}} + + # Wrong command arity must fail safely (no crash on KEYNUM keyspec path) + r acl setuser default on nopass resetkeys ~restricted:* +@all + catch {r aclcheck.rm_call.check.cmd eval script} e + assert_match {*DENIED*} $e } test {test module check acl for key prefix permission} { diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl index 911f11407c8..9c357f5c868 100644 --- a/tests/unit/scripting.tcl +++ b/tests/unit/scripting.tcl @@ -2686,3 +2686,15 @@ start_server {tags {"scripting"}} { } } } + +start_server {tags {"scripting"}} { + test "Wrong arity EVAL in acl_check_cmd returns error not crash" { + r acl setuser bob on {>123} {+@scripting} {+set} {~x*} + assert_equal [r auth bob 123] {OK} + # Must be the first Lua call in this server instance + catch {run_script { + return redis.acl_check_cmd('eval','script') + } 0} e + assert_match {*Wrong number of args*} $e + } +}