Skip to content

Commit 27a6d8a

Browse files
authored
fix(scripting): sanitize lua error replies (#3494)
fix #3493 Lua's error return poses an injection risk; see issues for details. I used codex gpt-5.5 to fix it.
1 parent 41ee6ae commit 27a6d8a

2 files changed

Lines changed: 22 additions & 1 deletion

File tree

src/storage/scripting.cc

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,16 @@ enum {
6060

6161
namespace lua {
6262

63+
namespace {
64+
65+
std::string SanitizeErrorMessage(std::string_view message) {
66+
std::string sanitized(message);
67+
std::erase_if(sanitized, [](char c) { return c == '\r' || c == '\n'; });
68+
return sanitized;
69+
}
70+
71+
} // namespace
72+
6373
lua_State *CreateState() {
6474
lua_State *lua = lua_open();
6575
LoadLibraries(lua);
@@ -1295,7 +1305,7 @@ std::string ReplyToRedisReply(redis::Connection *conn, lua_State *lua) {
12951305
lua_rawget(lua, -2);
12961306
t = lua_type(lua, -1);
12971307
if (t == LUA_TSTRING) {
1298-
output = redis::Error({Status::RedisErrorNoPrefix, lua_tostring(lua, -1)});
1308+
output = redis::Error({Status::RedisErrorNoPrefix, SanitizeErrorMessage(lua_tostring(lua, -1))});
12991309
lua_pop(lua, 1);
13001310
return output;
13011311
}

tests/gocase/unit/scripting/scripting_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,17 @@ func TestScripting(t *testing.T) {
9090
require.Nil(t, r.Val())
9191
})
9292

93+
t.Run("EVAL - Lua error reply strips embedded CRLF", func(t *testing.T) {
94+
c := srv.NewTCPClient()
95+
defer func() { require.NoError(t, c.Close()) }()
96+
97+
require.NoError(t, c.WriteArgs("EVAL", "return redis.error_reply('ERR injected\\r\\n+INJECTED\\r\\n')", "0"))
98+
c.MustRead(t, "-ERR injected+INJECTED")
99+
100+
require.NoError(t, c.WriteArgs("PING"))
101+
c.MustRead(t, "+PONG")
102+
})
103+
93104
t.Run("Script return recursive object", func(t *testing.T) {
94105
c := srv.NewTCPClient()
95106
defer func() { require.NoError(t, c.Close()) }()

0 commit comments

Comments
 (0)