File strip-CRLF-from-replies.patch of Package redis.42846
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(-)
diff --git a/src/functions.c b/src/functions.c
index c858db975bf..0d97d137d54 100644
--- a/src/functions.c
+++ b/src/functions.c
@@ -524,7 +524,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;
diff --git a/src/module.c b/src/module.c
index d58918201d7..37bc2db038f 100644
--- a/src/module.c
+++ b/src/module.c
@@ -3080,9 +3080,7 @@ int RM_ReplyWithErrorFormat(RedisModuleCtx *ctx, const char *fmt, ...) {
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;
}
diff --git a/src/networking.c b/src/networking.c
index bb1a72b9fb4..16406cd1a4c 100644
--- a/src/networking.c
+++ b/src/networking.c
@@ -624,6 +624,13 @@ void addReplyErrorSdsEx(client *c, sds err, int flags) {
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) {
@@ -690,11 +697,23 @@ void addReplyStatus(client *c, const char *status) {
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);
}
diff --git a/src/script_lua.c b/src/script_lua.c
index 8b64e624a60..3eccd70823e 100644
--- a/src/script_lua.c
+++ b/src/script_lua.c
@@ -650,10 +650,7 @@ static void luaReplyToRedisReply(client *c, client* script_client, lua_State *lu
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;
}
@@ -1754,7 +1751,7 @@ void luaCallFunction(scriptRunCtx* run_ctx, lua_State *lua, robj** keys, size_t
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 */
diff --git a/src/server.h b/src/server.h
index defcaf19c2c..5841c595a0c 100644
--- a/src/server.h
+++ b/src/server.h
@@ -2575,12 +2575,14 @@ void addReplyOrErrorObject(client *c, robj *reply);
void afterErrorReply(client *c, const char *s, size_t len, int flags);
void addReplyErrorFormatInternal(client *c, int flags, const char *fmt, va_list ap);
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);
diff --git a/tests/modules/reply.c b/tests/modules/reply.c
index c5baa66357a..7362cbff453 100644
--- a/tests/modules/reply.c
+++ b/tests/modules/reply.c
@@ -82,6 +82,19 @@ int rw_array(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
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);
@@ -193,6 +206,8 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
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)
diff --git a/tests/unit/functions.tcl b/tests/unit/functions.tcl
index 9e8ec08f327..1b5cbbe69e4 100644
--- a/tests/unit/functions.tcl
+++ b/tests/unit/functions.tcl
@@ -994,6 +994,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"}} {
diff --git a/tests/unit/moduleapi/reply.tcl b/tests/unit/moduleapi/reply.tcl
index 3cf284de374..54be2dc0cc0 100644
--- a/tests/unit/moduleapi/reply.tcl
+++ b/tests/unit/moduleapi/reply.tcl
@@ -143,6 +143,22 @@ start_server {tags {"modules"}} {
assert_match "WRONGTYPE A type error: foo5" $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
+ }
+
r hello 2
}
diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl
index d45c63ceec3..2901678f7bc 100644
--- a/tests/unit/scripting.tcl
+++ b/tests/unit/scripting.tcl
@@ -2311,6 +2311,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
