diff --git a/src/module.c b/src/module.c index dc17b40df..454042668 100644 --- a/src/module.c +++ b/src/module.c @@ -62,6 +62,7 @@ #include #include #include +#include /* -------------------------------------------------------------------------- * Private data structures used by the modules system. Those are data @@ -978,6 +979,26 @@ void RM_ChannelAtPosWithFlags(RedisModuleCtx *ctx, int pos, int flags) { res->numkeys++; } +/* Returns 1 if name is valid, otherwise returns 0. + * + * We want to block some chars in module command names that we know can + * mess things up. + * + * There are these characters: + * ' ' (space) - issues with old inline protocol. + * '\r', '\n' (newline) - can mess up the protocol on acl error replies. + * '|' - sub-commands. + * '@' - ACL categories. + * '=', ',' - info and client list fields (':' handled by getSafeInfoString). + * */ +int isCommandNameValid(const char *name) { + const char *block_chars = " \r\n|@=,"; + + if (strpbrk(name, block_chars)) + return 0; + return 1; +} + /* Helper for RM_CreateCommand(). Turns a string representing command * flags into the command flags used by the Redis core. * @@ -1019,9 +1040,14 @@ RedisModuleCommand *moduleCreateCommandProxy(struct RedisModule *module, sds dec /* Register a new command in the Redis server, that will be handled by * calling the function pointer 'cmdfunc' using the RedisModule calling - * convention. The function returns REDISMODULE_ERR if the specified command - * name is already busy or a set of invalid flags were passed, otherwise - * REDISMODULE_OK is returned and the new command is registered. + * convention. + * + * The function returns REDISMODULE_ERR in these cases: + * - The specified command is already busy. + * - The command name contains some chars that are not allowed. + * - A set of invalid flags were passed. + * + * Otherwise REDISMODULE_OK is returned and the new command is registered. * * This function must be called during the initialization of the module * inside the RedisModule_OnLoad() function. Calling this function outside @@ -1112,6 +1138,10 @@ int RM_CreateCommand(RedisModuleCtx *ctx, const char *name, RedisModuleCmdFunc c if ((flags & CMD_MODULE_NO_CLUSTER) && server.cluster_enabled) return REDISMODULE_ERR; + /* Check if the command name is valid. */ + if (!isCommandNameValid(name)) + return REDISMODULE_ERR; + /* Check if the command name is busy. */ if (lookupCommandByCString(name) != NULL) return REDISMODULE_ERR; @@ -1238,6 +1268,10 @@ int RM_CreateSubcommand(RedisModuleCommand *parent, const char *name, RedisModul if (parent_cp->func) return REDISMODULE_ERR; /* A parent command should be a pure container of subcommands */ + /* Check if the command name is valid. */ + if (!isCommandNameValid(name)) + return REDISMODULE_ERR; + /* Check if the command name is busy within the parent command. */ sds declared_name = sdsnew(name); if (parent_cmd->subcommands_dict && lookupSubcommand(parent_cmd, declared_name) != NULL) { diff --git a/tests/modules/subcommands.c b/tests/modules/subcommands.c index 3486e86b4..1b2bc5187 100644 --- a/tests/modules/subcommands.c +++ b/tests/modules/subcommands.c @@ -35,12 +35,23 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) if (RedisModule_Init(ctx, "subcommands", 1, REDISMODULE_APIVER_1) == REDISMODULE_ERR) return REDISMODULE_ERR; + /* Module command names cannot contain special characters. */ + RedisModule_Assert(RedisModule_CreateCommand(ctx,"subcommands.char\r",NULL,"",0,0,0) == REDISMODULE_ERR); + RedisModule_Assert(RedisModule_CreateCommand(ctx,"subcommands.char\n",NULL,"",0,0,0) == REDISMODULE_ERR); + RedisModule_Assert(RedisModule_CreateCommand(ctx,"subcommands.char ",NULL,"",0,0,0) == REDISMODULE_ERR); + if (RedisModule_CreateCommand(ctx,"subcommands.bitarray",NULL,"",0,0,0) == REDISMODULE_ERR) return REDISMODULE_ERR; RedisModuleCommand *parent = RedisModule_GetCommand(ctx,"subcommands.bitarray"); if (RedisModule_CreateSubcommand(parent,"set",cmd_set,"",0,0,0) == REDISMODULE_ERR) return REDISMODULE_ERR; + + /* Module subcommand names cannot contain special characters. */ + RedisModule_Assert(RedisModule_CreateSubcommand(parent,"char|",cmd_set,"",0,0,0) == REDISMODULE_ERR); + RedisModule_Assert(RedisModule_CreateSubcommand(parent,"char@",cmd_set,"",0,0,0) == REDISMODULE_ERR); + RedisModule_Assert(RedisModule_CreateSubcommand(parent,"char=",cmd_set,"",0,0,0) == REDISMODULE_ERR); + RedisModuleCommand *subcmd = RedisModule_GetCommand(ctx,"subcommands.bitarray|set"); RedisModuleCommandInfo cmd_set_info = { .version = REDISMODULE_COMMAND_INFO_VERSION,