This is an automated email from the ASF dual-hosted git repository.
shreemaan-abhishek pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix.git
The following commit(s) were added to refs/heads/master by this push:
new bf91696cc fix(xrpc): bound redis command-line preallocation size
(#13483)
bf91696cc is described below
commit bf91696cc1489670cd6e6ee4312c0550dff30945
Author: Shreemaan Abhishek <[email protected]>
AuthorDate: Wed Jun 10 16:06:13 2026 +0800
fix(xrpc): bound redis command-line preallocation size (#13483)
---
apisix/stream/xrpc/protocols/redis/init.lua | 28 ++++-
t/xrpc/redis.t | 152 ++++++++++++++++++++++++++++
2 files changed, 179 insertions(+), 1 deletion(-)
diff --git a/apisix/stream/xrpc/protocols/redis/init.lua
b/apisix/stream/xrpc/protocols/redis/init.lua
index 9aff6d0d1..73b09701a 100644
--- a/apisix/stream/xrpc/protocols/redis/init.lua
+++ b/apisix/stream/xrpc/protocols/redis/init.lua
@@ -21,12 +21,14 @@ local xrpc_socket =
require("resty.apisix.stream.xrpc.socket")
local ffi = require("ffi")
local ffi_str = ffi.string
local math_random = math.random
+local math_floor = math.floor
local OK = ngx.OK
local DECLINED = ngx.DECLINED
local DONE = ngx.DONE
local sleep = ngx.sleep
local str_byte = string.byte
local str_fmt = string.format
+local str_match = string.match
local ipairs = ipairs
local tonumber = tonumber
@@ -43,6 +45,7 @@ local protocol_name = "redis"
local _M = {}
local MAX_LINE_LEN = 128
local MAX_VALUE_LEN = 128
+local MAX_CMD_LINE_PREALLOC = 64
local PREFIX_ARR = str_byte("*")
local PREFIX_STR = str_byte("$")
local PREFIX_STA = str_byte("+")
@@ -134,6 +137,13 @@ local function read_len(sk)
end
local s = ffi_str(p + 1, len - 1)
+ -- RESP lengths are plain decimal integers; reject any other numeric
+ -- syntax (e.g. scientific notation like "1e9") so it cannot slip
+ -- through tonumber and be treated as a valid length.
+ if not str_match(s, "^%-?%d+$") then
+ return nil, str_fmt("invalid len string: \"%s\"", s)
+ end
+
local n = tonumber(s)
if not n then
return nil, str_fmt("invalid len string: \"%s\"", s)
@@ -148,13 +158,25 @@ local function read_req(session, sk)
return nil, err
end
- local cmd_line = core.tablepool.fetch("xrpc_redis_cmd_line", narg, 0)
+ if narg < 1 or narg ~= math_floor(narg) then
+ return nil, str_fmt("invalid argument number: %s", narg)
+ end
+
+ -- narg comes from the client; only use it as a bounded preallocation hint
+ -- so an oversized declared length cannot force a large upfront allocation.
+ -- the table still grows as the actual arguments are read.
+ local prealloc = narg < MAX_CMD_LINE_PREALLOC and narg or
MAX_CMD_LINE_PREALLOC
+ local cmd_line = core.tablepool.fetch("xrpc_redis_cmd_line", prealloc, 0)
local n, err = read_len(sk)
if not n then
return nil, err
end
+ if n < 0 then
+ return nil, str_fmt("invalid bulk length: %s", n)
+ end
+
local p, err = sk:read(n + 2)
if not p then
return nil, err
@@ -188,6 +210,10 @@ local function read_req(session, sk)
return nil, err
end
+ if n < 0 then
+ return nil, str_fmt("invalid bulk length: %s", n)
+ end
+
local s
if not is_key and n > MAX_VALUE_LEN then
-- avoid recording big value
diff --git a/t/xrpc/redis.t b/t/xrpc/redis.t
index cb438c1cc..f9cff50a7 100644
--- a/t/xrpc/redis.t
+++ b/t/xrpc/redis.t
@@ -781,3 +781,155 @@ log redis request ["psubscribe","dog*","cat*"]
log redis request ["publish","dog1","Hello"]
log redis request ["punsubscribe","cat*","dog*"]
log redis request ["set","dog","1"]
+
+
+
+=== TEST 14: invalid declared argument number is rejected
+--- config
+ location /t {
+ content_by_lua_block {
+ local sock = ngx.socket.tcp()
+ local ok, err = sock:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT)
+ if not ok then
+ ngx.say("failed to connect: ", err)
+ return
+ end
+
+ -- declare a negative argument count in the RESP array header
+ local bytes, err = sock:send("*-1\r\n")
+ if not bytes then
+ ngx.say("failed to send: ", err)
+ return
+ end
+
+ -- the connection should be closed by the gateway without a reply
+ local line = sock:receive("*a")
+ ngx.say("received: [", line or "", "]")
+ sock:close()
+ }
+ }
+--- response_body
+received: []
+--- stream_conf_enable
+--- error_log
+invalid argument number: -1
+--- no_error_log eval
+[
+ qr/\[error\](?!.*(?:invalid argument number|invalid len string|invalid
bulk length))/,
+ qr/\[crit\]/,
+ "stack traceback",
+]
+
+
+
+=== TEST 15: non-decimal declared length (scientific notation) is rejected
+--- config
+ location /t {
+ content_by_lua_block {
+ local sock = ngx.socket.tcp()
+ local ok, err = sock:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT)
+ if not ok then
+ ngx.say("failed to connect: ", err)
+ return
+ end
+
+ -- scientific notation parses with tonumber but is not valid RESP
+ local bytes, err = sock:send("*1e9\r\n")
+ if not bytes then
+ ngx.say("failed to send: ", err)
+ return
+ end
+
+ -- the connection should be closed by the gateway without a reply
+ local line = sock:receive("*a")
+ ngx.say("received: [", line or "", "]")
+ sock:close()
+ }
+ }
+--- response_body
+received: []
+--- stream_conf_enable
+--- error_log
+invalid len string: "1e9"
+--- no_error_log eval
+[
+ qr/\[error\](?!.*(?:invalid argument number|invalid len string|invalid
bulk length))/,
+ qr/\[crit\]/,
+ "stack traceback",
+]
+
+
+
+=== TEST 16: negative command bulk length is rejected
+--- config
+ location /t {
+ content_by_lua_block {
+ local sock = ngx.socket.tcp()
+ local ok, err = sock:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT)
+ if not ok then
+ ngx.say("failed to connect: ", err)
+ return
+ end
+
+ -- a valid array count but a negative bulk length for the command
+ local bytes, err = sock:send("*1\r\n$-1\r\n")
+ if not bytes then
+ ngx.say("failed to send: ", err)
+ return
+ end
+
+ -- the connection should be closed by the gateway without a reply
+ local line = sock:receive("*a")
+ ngx.say("received: [", line or "", "]")
+ sock:close()
+ }
+ }
+--- response_body
+received: []
+--- stream_conf_enable
+--- error_log
+invalid bulk length: -1
+--- no_error_log eval
+[
+ qr/\[error\](?!.*(?:invalid argument number|invalid len string|invalid
bulk length))/,
+ qr/\[crit\]/,
+ "stack traceback",
+]
+
+
+
+=== TEST 17: negative argument bulk length is rejected
+--- config
+ location /t {
+ content_by_lua_block {
+ local sock = ngx.socket.tcp()
+ local ok, err = sock:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT)
+ if not ok then
+ ngx.say("failed to connect: ", err)
+ return
+ end
+
+ -- valid command, then a negative bulk length for the argument
+ local bytes, err = sock:send("*2\r\n$3\r\nGET\r\n$-1\r\n")
+ if not bytes then
+ ngx.say("failed to send: ", err)
+ return
+ end
+
+ -- the connection should be closed by the gateway without a reply
+ local line = sock:receive("*a")
+ ngx.say("received: [", line or "", "]")
+ sock:close()
+ }
+ }
+--- response_body
+received: []
+--- stream_conf_enable
+--- error_log
+invalid bulk length: -1
+--- no_error_log eval
+[
+ qr/\[error\](?!.*(?:invalid argument number|invalid len string|invalid
bulk length))/,
+ qr/\[crit\]/,
+ "stack traceback",
+]
