diff --git a/src/networking.c b/src/networking.c index 73ea286e9..e0c2df37f 100644 --- a/src/networking.c +++ b/src/networking.c @@ -2585,6 +2585,12 @@ void resetCommand(client *c) { addReplyStatus(c,"RESET"); } +/* Disconnect the current client */ +void quitCommand(client *c) { + addReply(c,shared.ok); + c->flags |= CLIENT_CLOSE_AFTER_REPLY; +} + void clientCommand(client *c) { listNode *ln; listIter li; @@ -3193,7 +3199,7 @@ void helloCommand(client *c) { * when a POST or "Host:" header is seen, and will log the event from * time to time (to avoid creating a DOS as a result of too many logs). */ void securityWarningCommand(client *c) { - static time_t logged_time; + static time_t logged_time = 0; time_t now = time(NULL); if (llabs(now-logged_time) > 60) { diff --git a/src/server.c b/src/server.c index 09716f01e..2b789aec3 100644 --- a/src/server.c +++ b/src/server.c @@ -2008,12 +2008,6 @@ struct redisCommand redisCommandTable[] = { KSPEC_BS_INDEX,.bs.index={1}, KSPEC_FK_RANGE,.fk.range={0,1,0}}}}, - {"post",securityWarningCommand,-1, - "ok-loading ok-stale read-only"}, - - {"host:",securityWarningCommand,-1, - "ok-loading ok-stale read-only"}, - {"latency",NULL,-2, "", .subcommands=latencySubcommands}, @@ -2032,8 +2026,11 @@ struct redisCommand redisCommandTable[] = { KSPEC_FK_RANGE,.fk.range={1,1,0}}}, lcsGetKeys}, + {"quit",quitCommand,-1, + "no-auth no-script ok-stale ok-loading fast @connection"}, + {"reset",resetCommand,1, - "no-script ok-stale ok-loading fast @connection"}, + "no-auth no-script ok-stale ok-loading fast @connection"}, {"failover",failoverCommand,-1, "admin no-script ok-stale"} @@ -5187,13 +5184,9 @@ int processCommand(client *c) { moduleCallCommandFilters(c); - /* The QUIT command is handled separately. Normal command procs will - * go through checking for replication and QUIT will cause trouble - * when FORCE_REPLICATION is enabled and would be implemented in - * a regular command proc. */ - if (!strcasecmp(c->argv[0]->ptr,"quit")) { - addReply(c,shared.ok); - c->flags |= CLIENT_CLOSE_AFTER_REPLY; + /* Handle possible security attacks. */ + if (!strcasecmp(c->argv[0]->ptr,"host:") || !strcasecmp(c->argv[0]->ptr,"post")) { + securityWarningCommand(c); return C_ERR; } @@ -5338,6 +5331,7 @@ int processCommand(client *c) { if (c->flags & CLIENT_MULTI && c->cmd->proc != execCommand && c->cmd->proc != discardCommand && + c->cmd->proc != quitCommand && c->cmd->proc != resetCommand) { reject_cmd_on_oom = 1; } @@ -5405,6 +5399,7 @@ int processCommand(client *c) { c->cmd->proc != unsubscribeCommand && c->cmd->proc != psubscribeCommand && c->cmd->proc != punsubscribeCommand && + c->cmd->proc != quitCommand && c->cmd->proc != resetCommand) { rejectCommandFormat(c, "Can't execute '%s': only (P)SUBSCRIBE / " @@ -5445,6 +5440,7 @@ int processCommand(client *c) { c->cmd->proc != discardCommand && c->cmd->proc != watchCommand && c->cmd->proc != unwatchCommand && + c->cmd->proc != quitCommand && c->cmd->proc != resetCommand && !(c->cmd->proc == shutdownCommand && c->argc == 2 && @@ -5478,8 +5474,11 @@ int processCommand(client *c) { /* Exec the command */ if (c->flags & CLIENT_MULTI && - c->cmd->proc != execCommand && c->cmd->proc != discardCommand && - c->cmd->proc != multiCommand && c->cmd->proc != watchCommand && + c->cmd->proc != execCommand && + c->cmd->proc != discardCommand && + c->cmd->proc != multiCommand && + c->cmd->proc != watchCommand && + c->cmd->proc != quitCommand && c->cmd->proc != resetCommand) { queueMultiCommand(c); diff --git a/src/server.h b/src/server.h index d3ccdb080..11183d2c2 100644 --- a/src/server.h +++ b/src/server.h @@ -3015,6 +3015,7 @@ void xtrimCommand(client *c); void lolwutCommand(client *c); void aclCommand(client *c); void lcsCommand(client *c); +void quitCommand(client *c); void resetCommand(client *c); void failoverCommand(client *c); diff --git a/tests/modules/misc.c b/tests/modules/misc.c index 88039c6dd..daed69952 100644 --- a/tests/modules/misc.c +++ b/tests/modules/misc.c @@ -295,6 +295,14 @@ int test_log_tsctx(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) return REDISMODULE_OK; } +int test_weird_cmd(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { + REDISMODULE_NOT_USED(argv); + REDISMODULE_NOT_USED(argc); + + RedisModule_ReplyWithSimpleString(ctx, "OK"); + return REDISMODULE_OK; +} + int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { REDISMODULE_NOT_USED(argv); REDISMODULE_NOT_USED(argc); @@ -331,6 +339,9 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) return REDISMODULE_ERR; if (RedisModule_CreateCommand(ctx,"test.log_tsctx", test_log_tsctx,"",0,0,0) == REDISMODULE_ERR) return REDISMODULE_ERR; + /* Add a command with ':' in it's name, so that we can check commandstats sanitization. */ + if (RedisModule_CreateCommand(ctx,"test.weird:cmd", test_weird_cmd,"readonly",0,0,0) == REDISMODULE_ERR) + return REDISMODULE_ERR; return REDISMODULE_OK; } diff --git a/tests/unit/info.tcl b/tests/unit/info.tcl index 9c76f8f03..7e383bec3 100644 --- a/tests/unit/info.tcl +++ b/tests/unit/info.tcl @@ -164,12 +164,4 @@ start_server {tags {"info" "external:skip"}} { assert_match {} [errorstat NOPERM] } } - - start_server {} { - test {Unsafe command names are sanitized in INFO output} { - catch {r host:} e - set info [r info commandstats] - assert_match {*cmdstat_host_:calls=1*} $info - } - } } diff --git a/tests/unit/moduleapi/misc.tcl b/tests/unit/moduleapi/misc.tcl index a74c89aec..091de4a87 100644 --- a/tests/unit/moduleapi/misc.tcl +++ b/tests/unit/moduleapi/misc.tcl @@ -122,4 +122,10 @@ start_server {tags {"modules"}} { test {test RM_Call CLIENT INFO} { assert_match "*fd=-1*" [r test.call_generic client info] } + + test {Unsafe command names are sanitized in INFO output} { + r test.weird:cmd + set info [r info commandstats] + assert_match {*cmdstat_test.weird_cmd:calls=1*} $info + } }