Unverified Commit b161cff5 authored by guybe7's avatar guybe7 Committed by GitHub
Browse files

QUIT is a command, HOST: and POST are not (#9798)

Some people complain that QUIT is missing from help/command table.
Not appearing in COMMAND command, command stats, ACL, etc.
and instead, there's a hack in processCommand with a comment that looks outdated.
Note that it is [documented](https://redis.io/commands/quit)

At the same time, HOST: and POST are there in the command table although these are not real commands.
They would appear in the COMMAND command, and even in commandstats.

Other changes:
1. Initialize the static logged_time static var in securityWarningCommand
2. add `no-auth` flag to RESET so it can always be executed.
parent f07dedf7
...@@ -2585,6 +2585,12 @@ void resetCommand(client *c) { ...@@ -2585,6 +2585,12 @@ void resetCommand(client *c) {
addReplyStatus(c,"RESET"); 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) { void clientCommand(client *c) {
listNode *ln; listNode *ln;
listIter li; listIter li;
...@@ -3193,7 +3199,7 @@ void helloCommand(client *c) { ...@@ -3193,7 +3199,7 @@ void helloCommand(client *c) {
* when a POST or "Host:" header is seen, and will log the event from * 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). */ * time to time (to avoid creating a DOS as a result of too many logs). */
void securityWarningCommand(client *c) { void securityWarningCommand(client *c) {
static time_t logged_time; static time_t logged_time = 0;
time_t now = time(NULL); time_t now = time(NULL);
if (llabs(now-logged_time) > 60) { if (llabs(now-logged_time) > 60) {
......
...@@ -2008,12 +2008,6 @@ struct redisCommand redisCommandTable[] = { ...@@ -2008,12 +2008,6 @@ struct redisCommand redisCommandTable[] = {
KSPEC_BS_INDEX,.bs.index={1}, KSPEC_BS_INDEX,.bs.index={1},
KSPEC_FK_RANGE,.fk.range={0,1,0}}}}, 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, {"latency",NULL,-2,
"", "",
.subcommands=latencySubcommands}, .subcommands=latencySubcommands},
...@@ -2032,8 +2026,11 @@ struct redisCommand redisCommandTable[] = { ...@@ -2032,8 +2026,11 @@ struct redisCommand redisCommandTable[] = {
KSPEC_FK_RANGE,.fk.range={1,1,0}}}, KSPEC_FK_RANGE,.fk.range={1,1,0}}},
lcsGetKeys}, lcsGetKeys},
{"quit",quitCommand,-1,
"no-auth no-script ok-stale ok-loading fast @connection"},
{"reset",resetCommand,1, {"reset",resetCommand,1,
"no-script ok-stale ok-loading fast @connection"}, "no-auth no-script ok-stale ok-loading fast @connection"},
{"failover",failoverCommand,-1, {"failover",failoverCommand,-1,
"admin no-script ok-stale"} "admin no-script ok-stale"}
...@@ -5187,13 +5184,9 @@ int processCommand(client *c) { ...@@ -5187,13 +5184,9 @@ int processCommand(client *c) {
moduleCallCommandFilters(c); moduleCallCommandFilters(c);
/* The QUIT command is handled separately. Normal command procs will /* Handle possible security attacks. */
* go through checking for replication and QUIT will cause trouble if (!strcasecmp(c->argv[0]->ptr,"host:") || !strcasecmp(c->argv[0]->ptr,"post")) {
* when FORCE_REPLICATION is enabled and would be implemented in securityWarningCommand(c);
* a regular command proc. */
if (!strcasecmp(c->argv[0]->ptr,"quit")) {
addReply(c,shared.ok);
c->flags |= CLIENT_CLOSE_AFTER_REPLY;
return C_ERR; return C_ERR;
} }
...@@ -5338,6 +5331,7 @@ int processCommand(client *c) { ...@@ -5338,6 +5331,7 @@ int processCommand(client *c) {
if (c->flags & CLIENT_MULTI && if (c->flags & CLIENT_MULTI &&
c->cmd->proc != execCommand && c->cmd->proc != execCommand &&
c->cmd->proc != discardCommand && c->cmd->proc != discardCommand &&
c->cmd->proc != quitCommand &&
c->cmd->proc != resetCommand) { c->cmd->proc != resetCommand) {
reject_cmd_on_oom = 1; reject_cmd_on_oom = 1;
} }
...@@ -5405,6 +5399,7 @@ int processCommand(client *c) { ...@@ -5405,6 +5399,7 @@ int processCommand(client *c) {
c->cmd->proc != unsubscribeCommand && c->cmd->proc != unsubscribeCommand &&
c->cmd->proc != psubscribeCommand && c->cmd->proc != psubscribeCommand &&
c->cmd->proc != punsubscribeCommand && c->cmd->proc != punsubscribeCommand &&
c->cmd->proc != quitCommand &&
c->cmd->proc != resetCommand) { c->cmd->proc != resetCommand) {
rejectCommandFormat(c, rejectCommandFormat(c,
"Can't execute '%s': only (P)SUBSCRIBE / " "Can't execute '%s': only (P)SUBSCRIBE / "
...@@ -5445,6 +5440,7 @@ int processCommand(client *c) { ...@@ -5445,6 +5440,7 @@ int processCommand(client *c) {
c->cmd->proc != discardCommand && c->cmd->proc != discardCommand &&
c->cmd->proc != watchCommand && c->cmd->proc != watchCommand &&
c->cmd->proc != unwatchCommand && c->cmd->proc != unwatchCommand &&
c->cmd->proc != quitCommand &&
c->cmd->proc != resetCommand && c->cmd->proc != resetCommand &&
!(c->cmd->proc == shutdownCommand && !(c->cmd->proc == shutdownCommand &&
c->argc == 2 && c->argc == 2 &&
...@@ -5478,8 +5474,11 @@ int processCommand(client *c) { ...@@ -5478,8 +5474,11 @@ int processCommand(client *c) {
/* Exec the command */ /* Exec the command */
if (c->flags & CLIENT_MULTI && if (c->flags & CLIENT_MULTI &&
c->cmd->proc != execCommand && c->cmd->proc != discardCommand && c->cmd->proc != execCommand &&
c->cmd->proc != multiCommand && c->cmd->proc != watchCommand && c->cmd->proc != discardCommand &&
c->cmd->proc != multiCommand &&
c->cmd->proc != watchCommand &&
c->cmd->proc != quitCommand &&
c->cmd->proc != resetCommand) c->cmd->proc != resetCommand)
{ {
queueMultiCommand(c); queueMultiCommand(c);
......
...@@ -3015,6 +3015,7 @@ void xtrimCommand(client *c); ...@@ -3015,6 +3015,7 @@ void xtrimCommand(client *c);
void lolwutCommand(client *c); void lolwutCommand(client *c);
void aclCommand(client *c); void aclCommand(client *c);
void lcsCommand(client *c); void lcsCommand(client *c);
void quitCommand(client *c);
void resetCommand(client *c); void resetCommand(client *c);
void failoverCommand(client *c); void failoverCommand(client *c);
......
...@@ -295,6 +295,14 @@ int test_log_tsctx(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) ...@@ -295,6 +295,14 @@ int test_log_tsctx(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
return REDISMODULE_OK; 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) { int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
REDISMODULE_NOT_USED(argv); REDISMODULE_NOT_USED(argv);
REDISMODULE_NOT_USED(argc); REDISMODULE_NOT_USED(argc);
...@@ -331,6 +339,9 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) ...@@ -331,6 +339,9 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
return REDISMODULE_ERR; return REDISMODULE_ERR;
if (RedisModule_CreateCommand(ctx,"test.log_tsctx", test_log_tsctx,"",0,0,0) == REDISMODULE_ERR) if (RedisModule_CreateCommand(ctx,"test.log_tsctx", test_log_tsctx,"",0,0,0) == REDISMODULE_ERR)
return 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; return REDISMODULE_OK;
} }
...@@ -164,12 +164,4 @@ start_server {tags {"info" "external:skip"}} { ...@@ -164,12 +164,4 @@ start_server {tags {"info" "external:skip"}} {
assert_match {} [errorstat NOPERM] 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
}
}
} }
...@@ -122,4 +122,10 @@ start_server {tags {"modules"}} { ...@@ -122,4 +122,10 @@ start_server {tags {"modules"}} {
test {test RM_Call CLIENT INFO} { test {test RM_Call CLIENT INFO} {
assert_match "*fd=-1*" [r test.call_generic 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
}
} }
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment