From d90b3aee684c1863b31a5b9be2b9d0b3ade93c7f Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Fri, 19 Jun 2026 12:06:36 +0800 Subject: [PATCH 1/4] fix(limit-count): validate variable-resolved count/time_window bounds count and time_window may be set from request variables (e.g. "${http_count}"). The resolved value was only coerced with tonumber, so a client could supply 0, a negative, a fractional, or an out-of-range value, which slipped past the schema's >0 integer bounds and hit the limiter constructor's assert (500) or skewed the limit. Validate the resolved value the same way limit-conn already does: positive, integer, within safe integer range. --- apisix/plugins/limit-count/init.lua | 15 ++++++- t/plugin/limit-count-variable.t | 69 +++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/apisix/plugins/limit-count/init.lua b/apisix/plugins/limit-count/init.lua index ed4db64a8cad..8e58eaed887c 100644 --- a/apisix/plugins/limit-count/init.lua +++ b/apisix/plugins/limit-count/init.lua @@ -389,9 +389,22 @@ local function resolve_var(ctx, value) if err then return nil, "could not resolve var for value: " .. original_value .. ", err: " .. err end + local resolved = value value = tonumber(value) if not value then - return nil, "resolved value is not a number: " .. tostring(value) + return nil, "resolved value is not a number: " .. tostring(resolved) + end + -- count/time_window must be positive integers, matching the schema + if value <= 0 then + return nil, "resolved value must be a positive number, got: " .. tostring(value) + end + if value ~= math_floor(value) then + return nil, "resolved value must be an integer, got: " .. tostring(value) + end + -- LuaJIT doubles lose integer precision above 2^53 + if value > 9007199254740991 then + return nil, "resolved value exceeds safe integer range (2^53-1), got: " + .. tostring(value) end end return value diff --git a/t/plugin/limit-count-variable.t b/t/plugin/limit-count-variable.t index a4359c63a87b..d9bbde9ce515 100644 --- a/t/plugin/limit-count-variable.t +++ b/t/plugin/limit-count-variable.t @@ -284,3 +284,72 @@ nginx_config: --- error_code: 200 --- access_log eval qr/\{\\x22rate_limiting_key\\x22:\\x22\/apisix\/routes\/1:\d+:test\.com\\x22,\\x22rate_limiting_limit\\x22:2,\\x22rate_limiting_remaining\\x22:1,\\x22rate_limiting_reset\\x22:10}/ + + + +=== TEST 9: reject out-of-bounds resolved count/time_window +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "methods": ["GET"], + "plugins": { + "limit-count": { + "count": "${http_count ?? 2}", + "time_window": "${http_time_window ?? 5}", + "rejected_code": 503, + "key_type": "var", + "key": "remote_addr", + "policy": "local" + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 10: a client-supplied 0/negative/fractional count is rejected, not bypassed +--- config + location /t { + content_by_lua_block { + local http = require("resty.http") + local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" + local httpc = http.new() + for _, count in ipairs({"0", "-1", "1.5", "9999999999999999"}) do + local res = httpc:request_uri(uri, {method = "GET", + headers = {["count"] = count}}) + if res.status ~= 500 then + ngx.say("count=", count, " should be rejected with 500, got ", res.status) + return + end + end + ngx.say("passed") + } + } +--- request +GET /t +--- response_body +passed +--- error_log +resolved value must be a positive number +resolved value must be an integer +resolved value exceeds safe integer range From e981b971a84c16b50b83be91e8ed7cdce04b871e Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 22 Jun 2026 10:13:46 +0800 Subject: [PATCH 2/4] test(limit-count): cover time_window var bounds; drop raw value from error Address review: use a generic error message instead of echoing the raw resolved value (request-derived), clarify TEST 9 as route setup, and add TEST 11 exercising out-of-bounds time_window rejection. --- apisix/plugins/limit-count/init.lua | 3 +-- t/plugin/limit-count-variable.t | 32 ++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/apisix/plugins/limit-count/init.lua b/apisix/plugins/limit-count/init.lua index 8e58eaed887c..64cd6e425a16 100644 --- a/apisix/plugins/limit-count/init.lua +++ b/apisix/plugins/limit-count/init.lua @@ -389,10 +389,9 @@ local function resolve_var(ctx, value) if err then return nil, "could not resolve var for value: " .. original_value .. ", err: " .. err end - local resolved = value value = tonumber(value) if not value then - return nil, "resolved value is not a number: " .. tostring(resolved) + return nil, "resolved value is not a number" end -- count/time_window must be positive integers, matching the schema if value <= 0 then diff --git a/t/plugin/limit-count-variable.t b/t/plugin/limit-count-variable.t index d9bbde9ce515..72344f49bf52 100644 --- a/t/plugin/limit-count-variable.t +++ b/t/plugin/limit-count-variable.t @@ -287,7 +287,7 @@ qr/\{\\x22rate_limiting_key\\x22:\\x22\/apisix\/routes\/1:\d+:test\.com\\x22,\\x -=== TEST 9: reject out-of-bounds resolved count/time_window +=== TEST 9: set up route with count/time_window from request variables --- config location /t { content_by_lua_block { @@ -353,3 +353,33 @@ passed resolved value must be a positive number resolved value must be an integer resolved value exceeds safe integer range + + + +=== TEST 11: a client-supplied 0/negative/fractional time_window is rejected, not bypassed +--- config + location /t { + content_by_lua_block { + local http = require("resty.http") + local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" + local httpc = http.new() + for _, time_window in ipairs({"0", "-1", "1.5", "9999999999999999"}) do + local res = httpc:request_uri(uri, {method = "GET", + headers = {["time_window"] = time_window}}) + if res.status ~= 500 then + ngx.say("time_window=", time_window, " should be rejected with 500, got ", + res.status) + return + end + end + ngx.say("passed") + } + } +--- request +GET /t +--- response_body +passed +--- error_log +resolved value must be a positive number +resolved value must be an integer +resolved value exceeds safe integer range From 601b7dd7e6980ee78c1ce8a7ada6a9ed843899b4 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 22 Jun 2026 18:56:44 +0800 Subject: [PATCH 3/4] fix(limit-count): reject invalid count/time_window in rules mode In rules mode get_rules() swallowed resolve_var errors via goto CONTINUE, so a client-controlled variable resolving to an invalid count/time_window silently dropped the rule instead of rejecting the request, bypassing the limit. Return the error for count/time_window; only skip a rule when its key does not resolve. Add a rules-mode regression test. --- apisix/plugins/limit-count/init.lua | 7 ++- t/plugin/limit-count-variable.t | 69 +++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/apisix/plugins/limit-count/init.lua b/apisix/plugins/limit-count/init.lua index 64cd6e425a16..2f0412260639 100644 --- a/apisix/plugins/limit-count/init.lua +++ b/apisix/plugins/limit-count/init.lua @@ -432,14 +432,17 @@ local function get_rules(ctx, conf) local rules = {} for index, rule in ipairs(conf.rules) do + -- an invalid count/time_window must reject, not silently skip the rule, + -- otherwise a client-controlled variable could disable rate limiting local count, err = resolve_var(ctx, rule.count) if err then - goto CONTINUE + return nil, err end local time_window, err2 = resolve_var(ctx, rule.time_window) if err2 then - goto CONTINUE + return nil, err2 end + -- a rule keyed on a var absent for this request just doesn't apply local key, _, n_resolved = core.utils.resolve_var(rule.key, ctx.var) if n_resolved == 0 then goto CONTINUE diff --git a/t/plugin/limit-count-variable.t b/t/plugin/limit-count-variable.t index 72344f49bf52..e879a871c8b5 100644 --- a/t/plugin/limit-count-variable.t +++ b/t/plugin/limit-count-variable.t @@ -383,3 +383,72 @@ passed resolved value must be a positive number resolved value must be an integer resolved value exceeds safe integer range + + + +=== TEST 12: set up rules-mode route with count from a request variable +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "methods": ["GET"], + "plugins": { + "limit-count": { + "rejected_code": 503, + "rules": [ + { + "key": "${http_user}", + "count": "${http_count ?? 2}", + "time_window": 60 + } + ] + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 13: rules-mode invalid count rejects, not silently skips the rule +--- config + location /t { + content_by_lua_block { + local http = require("resty.http") + local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" + local httpc = http.new() + -- the rule's key resolves (user header present), so without bounds + -- validation a count of 0 would drop the rule and let the request pass + local res = httpc:request_uri(uri, {method = "GET", + headers = {["user"] = "jack", ["count"] = "0"}}) + if res.status ~= 500 then + ngx.say("invalid rule count should be rejected with 500, got ", res.status) + return + end + ngx.say("passed") + } + } +--- request +GET /t +--- response_body +passed +--- error_log +resolved value must be a positive number From e65e82c238e76589d3e6447bf782390827d7bc7b Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Tue, 23 Jun 2026 10:09:22 +0800 Subject: [PATCH 4/4] fix(limit-count): skip non-applying rules before validating count/time_window A rule whose key resolves to nothing must be skipped before its count/ time_window are validated, otherwise a rule meant not to apply (its count/time_window vars also absent) wrongly rejects the whole request. --- apisix/plugins/limit-count/init.lua | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/apisix/plugins/limit-count/init.lua b/apisix/plugins/limit-count/init.lua index 2f0412260639..838bf27fdb56 100644 --- a/apisix/plugins/limit-count/init.lua +++ b/apisix/plugins/limit-count/init.lua @@ -432,8 +432,13 @@ local function get_rules(ctx, conf) local rules = {} for index, rule in ipairs(conf.rules) do - -- an invalid count/time_window must reject, not silently skip the rule, - -- otherwise a client-controlled variable could disable rate limiting + -- a rule keyed on a var absent for this request just doesn't apply + local key, _, n_resolved = core.utils.resolve_var(rule.key, ctx.var) + if n_resolved == 0 then + goto CONTINUE + end + -- the rule applies, so an invalid count/time_window must reject, not + -- silently skip it, else a client-controlled var could disable limiting local count, err = resolve_var(ctx, rule.count) if err then return nil, err @@ -442,11 +447,6 @@ local function get_rules(ctx, conf) if err2 then return nil, err2 end - -- a rule keyed on a var absent for this request just doesn't apply - local key, _, n_resolved = core.utils.resolve_var(rule.key, ctx.var) - if n_resolved == 0 then - goto CONTINUE - end core.table.insert(rules, { count = count, time_window = time_window,