File strip-CRLF-from-replies.patch of Package redis7.42847
From 857e9ac483b0f18b5ab60a7de2bbc248844dc772 Mon Sep 17 00:00:00 2001
From: "debing.sun" <debing.sun@redis.com>
Date: Sun, 28 Dec 2025 15:37:48 +0800
Subject: [PATCH] Strip CRLF from error and simple string replies (#826)
Because in some cases, the client put \r\n in the command parameters.
When Redis returns these parameters to the client via an error reply, the presence of \r\n in the middle can cause the client to only parse the portion before the \r\n when handling the error reply. This disrupts the protocol parsing and ultimately causes the connection to become stuck.
---
src/functions.c | 2 +-
src/module.c | 4 +---
src/networking.c | 19 +++++++++++++++++++
src/script_lua.c | 7 ++-----
src/server.h | 2 ++
tests/modules/reply.c | 15 +++++++++++++++
tests/unit/functions.tcl | 9 +++++++++
tests/unit/moduleapi/reply.tcl | 16 ++++++++++++++++
tests/unit/scripting.tcl | 8 ++++++++
9 files changed, 73 insertions(+), 9 deletions(-)
Index: b/src/functions.c
===================================================================
--- a/src/functions.c
+++ b/src/functions.c
@@ -522,7 +522,7 @@ void functionListCommand(client *c) {
library_name = c->argv[++i]->ptr;
continue;
}
- addReplyErrorSds(c, sdscatfmt(sdsempty(), "Unknown argument %s", next_arg->ptr));
+ addReplyErrorSdsSafe(c, sdscatfmt(sdsempty(), "Unknown argument %s", next_arg->ptr));
return;
}
size_t reply_len = 0;
Index: b/src/module.c
===================================================================
--- a/src/module.c
+++ b/src/module.c
@@ -2755,9 +2755,7 @@ int RM_ReplyWithError(RedisModuleCtx *ct
int RM_ReplyWithSimpleString(RedisModuleCtx *ctx, const char *msg) {
client *c = moduleGetReplyClient(ctx);
if (c == NULL) return REDISMODULE_OK;
- addReplyProto(c,"+",1);
- addReplyProto(c,msg,strlen(msg));
- addReplyProto(c,"\r\n",2);
+ addReplyStatusSafe(c, msg);
return REDISMODULE_OK;
}
Index: b/src/networking.c
===================================================================
--- a/src/networking.c
+++ b/src/networking.c
@@ -595,6 +595,13 @@ void addReplyErrorSdsEx(client *c, sds e
sdsfree(err);
}
+/* See addReplyErrorLength for expectations from the input string.
+ * The string is safe to contain \r and \n anywhere. */
+void addReplyErrorSdsExSafe(client *c, sds err, int flags) {
+ err = sdsmapchars(err, "\r\n", " ", 2);
+ addReplyErrorSdsEx(c, err, flags);
+}
+
/* See addReplyErrorLength for expectations from the input string. */
/* As a side effect the SDS string is freed. */
void addReplyErrorSds(client *c, sds err) {
@@ -661,11 +668,23 @@ void addReplyStatus(client *c, const cha
addReplyStatusLength(c,status,strlen(status));
}
+/* Like addReplyStatus() but the string is safe to contain \r and \n anywhere. */
+void addReplyStatusSafe(client *c, const char *status) {
+ sds s = sdsmapchars(sdsnew(status), "\r\n", " ", 2);
+ addReplyStatusLength(c,s,sdslen(s));
+ sdsfree(s);
+}
+
void addReplyStatusFormat(client *c, const char *fmt, ...) {
va_list ap;
va_start(ap,fmt);
sds s = sdscatvprintf(sdsempty(),fmt,ap);
va_end(ap);
+ /* Trim any newlines at the end (ones will be added by addReplyStatusLength) */
+ s = sdstrim(s, "\r\n");
+ /* Make sure there are no newlines in the middle of the string, otherwise
+ * invalid protocol is emitted. */
+ s = sdsmapchars(s, "\r\n", " ", 2);
addReplyStatusLength(c,s,sdslen(s));
sdsfree(s);
}
Index: b/src/script_lua.c
===================================================================
--- a/src/script_lua.c
+++ b/src/script_lua.c
@@ -648,10 +648,7 @@ static void luaReplyToRedisReply(client
lua_rawget(lua,-2);
t = lua_type(lua,-1);
if (t == LUA_TSTRING) {
- sds ok = sdsnew(lua_tostring(lua,-1));
- sdsmapchars(ok,"\r\n"," ",2);
- addReplyStatusLength(c, ok, sdslen(ok));
- sdsfree(ok);
+ addReplyStatusSafe(c, lua_tostring(lua,-1));
lua_pop(lua,2);
return;
}
@@ -1764,7 +1761,7 @@ void luaCallFunction(scriptRunCtx* run_c
err_info.source,
err_info.line);
}
- addReplyErrorSdsEx(c, final_msg, err_info.ignore_err_stats_update? ERR_REPLY_FLAG_NO_STATS_UPDATE : 0);
+ addReplyErrorSdsExSafe(c, final_msg, err_info.ignore_err_stats_update? ERR_REPLY_FLAG_NO_STATS_UPDATE : 0);
luaErrorInformationDiscard(&err_info);
}
lua_pop(lua,1); /* Consume the Lua error */
Index: b/src/server.h
===================================================================
--- a/src/server.h
+++ b/src/server.h
@@ -2476,12 +2476,14 @@ void addReplyErrorObject(client *c, robj
void addReplyOrErrorObject(client *c, robj *reply);
void afterErrorReply(client *c, const char *s, size_t len, int flags);
void addReplyErrorSdsEx(client *c, sds err, int flags);
+void addReplyErrorSdsExSafe(client *c, sds err, int flags);
void addReplyErrorSds(client *c, sds err);
void addReplyErrorSdsSafe(client *c, sds err);
void addReplyError(client *c, const char *err);
void addReplyErrorArity(client *c);
void addReplyErrorExpireTime(client *c);
void addReplyStatus(client *c, const char *status);
+void addReplyStatusSafe(client *c, const char *s);
void addReplyDouble(client *c, double d);
void addReplyLongLongWithPrefix(client *c, long long ll, char prefix);
void addReplyBigNum(client *c, const char* num, size_t len);
Index: b/tests/modules/reply.c
===================================================================
--- a/tests/modules/reply.c
+++ b/tests/modules/reply.c
@@ -71,6 +71,19 @@ int rw_array(RedisModuleCtx *ctx, RedisM
return REDISMODULE_OK;
}
+int rw_simplestring_array(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
+ if (argc < 2) return RedisModule_WrongArity(ctx);
+
+ RedisModule_ReplyWithArray(ctx, argc - 1);
+ for (int i = 1; i < argc; ++i) {
+ size_t len;
+ const char *str = RedisModule_StringPtrLen(argv[i], &len);
+ RedisModule_ReplyWithSimpleString(ctx, str);
+ }
+
+ return REDISMODULE_OK;
+}
+
int rw_map(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
if (argc != 2) return RedisModule_WrongArity(ctx);
@@ -174,6 +187,8 @@ int RedisModule_OnLoad(RedisModuleCtx *c
return REDISMODULE_ERR;
if (RedisModule_CreateCommand(ctx,"rw.array",rw_array,"",0,0,0) != REDISMODULE_OK)
return REDISMODULE_ERR;
+ if (RedisModule_CreateCommand(ctx,"rw.simplestring_array",rw_simplestring_array,"",0,0,0) != REDISMODULE_OK)
+ return REDISMODULE_ERR;
if (RedisModule_CreateCommand(ctx,"rw.map",rw_map,"",0,0,0) != REDISMODULE_OK)
return REDISMODULE_ERR;
if (RedisModule_CreateCommand(ctx,"rw.attribute",rw_attribute,"",0,0,0) != REDISMODULE_OK)
Index: b/tests/unit/functions.tcl
===================================================================
--- a/tests/unit/functions.tcl
+++ b/tests/unit/functions.tcl
@@ -985,6 +985,15 @@ start_server {tags {"scripting"}} {
r config set maxmemory 0
} {OK} {needs:config-maxmemory}
+
+ test {FUNCTION - test function list with CRLF injection attempt} {
+ r function flush
+
+ # This test verifies that CRLF characters are properly handled
+ # to prevent protocol injection attacks when using FUNCTION LIST.
+ # The error message should have CRLF replaced with spaces.
+ assert_error {*Unknown argument X +INJECTED*} {r function list "X\r\n+INJECTED"}
+ }
}
start_server {tags {"scripting"}} {
Index: b/tests/unit/moduleapi/reply.tcl
===================================================================
--- a/tests/unit/moduleapi/reply.tcl
+++ b/tests/unit/moduleapi/reply.tcl
@@ -93,6 +93,22 @@ start_server {tags {"modules"}} {
catch {r rw.error} e
assert_match "An error" $e
}
+
+ test "RESP$proto: redis.call reply parsing with invalid CRLF character" {
+ # When Lua parses redis.call replies, the current implementation only
+ # searches for '\r' characters without verifying that '\n' follows. If a '\r'
+ # appears in the protocol data (not as part of the CRLF delimiter), the parser
+ # incorrectly treats it as a valid '\r\n' terminator.
+ #
+ # Example: Protocol data containing "\rx=100000000000" would be parsed as:
+ # - '\rx' is treated as line terminator (should require '\r\n')
+ # - '=100000000000' is interpreted as length specifier
+ # - Lua attempts to create a massive string → Out of Memory
+ r deferred 1
+ r eval {return redis.call('rw.simplestring_array', '\rx=100000000000', 'hello')} 0
+ assert_equal [r rawread 30] "*2\r\n+ x=100000000000\r\n+hello\r\n"
+ r deferred 0
+ }
}
test "Unload the module - replywith" {
Index: b/tests/unit/scripting.tcl
===================================================================
--- a/tests/unit/scripting.tcl
+++ b/tests/unit/scripting.tcl
@@ -2120,6 +2120,14 @@ start_server {tags {"scripting"}} {
assert_equal [errorrstat ERR r] {count=1}
}
+ test "LUA redis.error_reply API with CRLF injection attempt" {
+ catch {
+ r eval {error(redis.error_reply("X\r\n+INJECTED"))} 0
+ } err
+ # The error message should have CRLF replaced with spaces
+ assert_match {ERR X +INJECTED*} $err
+ }
+
test "LUA redis.status_reply API" {
r config resetstat
r readraw 1
