From 3e7d59eb62eda88e3eb6112c7cdaf207e63fbcce Mon Sep 17 00:00:00 2001 From: Felipe Zipitria Date: Sun, 1 Nov 2026 12:11:07 -0800 Subject: [PATCH 1/2] fix: Lua m.setvar() returned -1, handing the script stack garbage Lua::setvar() returned -1 on its error paths (wrong number of arguments, no collection in the name). A lua_CFunction returns the number of values it pushed, so Lua returned whatever was on the stack: the script got the __transaction pointer or a stray string. Return 0 so the script gets nil. Also call luaL_checkstring() before the std::string locals exist, since it raises a Lua error with longjmp on a non-string argument, drop the NULL check it makes unreachable, and stop popping the arguments while var_value still points into them. Fixes #3731 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RiUJuqXNvvpytiGRiDFyTy --- src/engine/lua.cc | 25 +++--- test/test-cases/data/setvar-error-nodot.lua | 5 ++ test/test-cases/data/setvar-error-onearg.lua | 5 ++ .../test-cases/regression/collection-lua.json | 86 +++++++++++++++++++ 4 files changed, 108 insertions(+), 13 deletions(-) create mode 100644 test/test-cases/data/setvar-error-nodot.lua create mode 100644 test/test-cases/data/setvar-error-onearg.lua diff --git a/src/engine/lua.cc b/src/engine/lua.cc index b256b57d68..ed5ce41f9a 100644 --- a/src/engine/lua.cc +++ b/src/engine/lua.cc @@ -325,9 +325,6 @@ int Lua::setvar(lua_State *L) { Transaction *t(NULL); const char *var_value(NULL); const char *var_name(NULL); - std::string vname; - std::string collection; - std::string variableName; int nargs = lua_gettop(L); size_t pos; void *z(NULL); @@ -337,21 +334,23 @@ int Lua::setvar(lua_State *L) { t = reinterpret_cast(z); + /* A lua_CFunction returns the number of values it pushed, so the + * error paths return 0 (the script gets nil), never a negative value. + */ if (nargs != 2) { ms_dbg_a(t, 8, - "m.setvar: Failed m.setvar funtion must has 2 arguments"); - return -1; + "m.setvar: Failed m.setvar function must have 2 arguments"); + return 0; } + /* luaL_checkstring() raises a Lua error (longjmp) on a non-string + * argument, so call it before any C++ object with a destructor exists. + */ var_value = luaL_checkstring(L, 2); var_name = luaL_checkstring(L, 1); - lua_pop(L, 2); - - if (var_value == NULL || var_name == NULL) { - return -1; - } - - vname.assign(var_name); + std::string vname(var_name); + std::string collection; + std::string variableName; pos = vname.find("."); if (pos != std::string::npos) { collection = std::string(vname, 0, pos); @@ -363,7 +362,7 @@ int Lua::setvar(lua_State *L) { ms_dbg_a(t, 8, "m.setvar: Must specify a collection using dot character" \ " - ie m.setvar(tx.myvar,mydata)"); - return -1; + return 0; } if (collection == "TX") { diff --git a/test/test-cases/data/setvar-error-nodot.lua b/test/test-cases/data/setvar-error-nodot.lua new file mode 100644 index 0000000000..06adaa4220 --- /dev/null +++ b/test/test-cases/data/setvar-error-nodot.lua @@ -0,0 +1,5 @@ +function main() + local r = m.setvar("novar", "x"); + m.log(4, "m.setvar() without a collection returned: " .. tostring(r)); + return nil; +end diff --git a/test/test-cases/data/setvar-error-onearg.lua b/test/test-cases/data/setvar-error-onearg.lua new file mode 100644 index 0000000000..6b200fe852 --- /dev/null +++ b/test/test-cases/data/setvar-error-onearg.lua @@ -0,0 +1,5 @@ +function main() + local r = m.setvar("tx.a"); + m.log(4, "m.setvar() with one argument returned: " .. tostring(r)); + return nil; +end diff --git a/test/test-cases/regression/collection-lua.json b/test/test-cases/regression/collection-lua.json index 1c20b35f5a..03c0969478 100644 --- a/test/test-cases/regression/collection-lua.json +++ b/test/test-cases/regression/collection-lua.json @@ -310,5 +310,91 @@ "SecRuleEngine On", "SecRuleScript test-cases/data/match-getvars-args.lua \"id:2,phase:2,deny,status:403\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing LUA :: m.setvar without a collection returns nil", + "resource": "lua", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "My sweet little browser", + "Accept": "*/*", + "Content-Length": "0" + }, + "uri": "/whee?res=1", + "method": "GET", + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "m.setvar\\(\\) without a collection returned: nil", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRuleScript test-cases/data/setvar-error-nodot.lua \"id:1,pass\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing LUA :: m.setvar with one argument returns nil", + "resource": "lua", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "My sweet little browser", + "Accept": "*/*", + "Content-Length": "0" + }, + "uri": "/whee?res=1", + "method": "GET", + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "m.setvar\\(\\) with one argument returned: nil", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRuleScript test-cases/data/setvar-error-onearg.lua \"id:1,pass\"" + ] } ] From 3b81dca151b437f98380a8bd9ed00038228d6d4a Mon Sep 17 00:00:00 2001 From: Felipe Zipitria Date: Sun, 1 Nov 2026 17:16:27 -0800 Subject: [PATCH 2/2] fix: log an unknown collection in Lua m.setvar() m.setvar("FOO.bar", "x") fell through every collection branch and was ignored without a message. Log it at level 8, like the other m.setvar() errors, with the name escaped. Tests: assert the debug message on each error path as well as the nil return, since m.setvar() also returns nil on success. Add cases for an unknown collection, a successful TX set (both from @airween), and a non-string value, which must fail the script with a Lua error. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RiUJuqXNvvpytiGRiDFyTy --- src/engine/lua.cc | 4 + test/test-cases/data/setvar-error-table.lua | 5 + .../data/setvar-error-unknown-collection.lua | 5 + test/test-cases/data/setvar-tx.lua | 6 + .../test-cases/regression/collection-lua.json | 133 +++++++++++++++++- 5 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 test/test-cases/data/setvar-error-table.lua create mode 100644 test/test-cases/data/setvar-error-unknown-collection.lua create mode 100644 test/test-cases/data/setvar-tx.lua diff --git a/src/engine/lua.cc b/src/engine/lua.cc index ed5ce41f9a..0aca87093c 100644 --- a/src/engine/lua.cc +++ b/src/engine/lua.cc @@ -402,6 +402,10 @@ int Lua::setvar(lua_State *L) { var_value); } + else { + ms_dbg_a(t, 8, "m.setvar: Unknown collection: " + + utils::string::toHexIfNeeded(collection) + "."); + } return 0; } diff --git a/test/test-cases/data/setvar-error-table.lua b/test/test-cases/data/setvar-error-table.lua new file mode 100644 index 0000000000..89b1c1def2 --- /dev/null +++ b/test/test-cases/data/setvar-error-table.lua @@ -0,0 +1,5 @@ +function main() + m.setvar("tx.a", {}); + m.log(4, "not reached"); + return nil; +end diff --git a/test/test-cases/data/setvar-error-unknown-collection.lua b/test/test-cases/data/setvar-error-unknown-collection.lua new file mode 100644 index 0000000000..6e1683812a --- /dev/null +++ b/test/test-cases/data/setvar-error-unknown-collection.lua @@ -0,0 +1,5 @@ +function main() + local r = m.setvar("FOO.bar", "x"); + m.log(4, "m.setvar() with an unknown collection returned: " .. tostring(r)); + return nil; +end diff --git a/test/test-cases/data/setvar-tx.lua b/test/test-cases/data/setvar-tx.lua new file mode 100644 index 0000000000..092fd88d64 --- /dev/null +++ b/test/test-cases/data/setvar-tx.lua @@ -0,0 +1,6 @@ +function main() + local r = m.setvar("tx.a", "x"); + m.log(4, "m.setvar() with TX returned: " .. tostring(r)); + m.log(4, "tx.a is now: " .. tostring(m.getvar("tx.a"))); + return nil; +end diff --git a/test/test-cases/regression/collection-lua.json b/test/test-cases/regression/collection-lua.json index 03c0969478..5f5047ffca 100644 --- a/test/test-cases/regression/collection-lua.json +++ b/test/test-cases/regression/collection-lua.json @@ -346,7 +346,7 @@ ] }, "expected": { - "debug_log": "m.setvar\\(\\) without a collection returned: nil", + "debug_log": "m.setvar: Must specify a collection using dot character.*m.setvar\\(\\) without a collection returned: nil", "http_code": 200 }, "rules": [ @@ -389,12 +389,141 @@ ] }, "expected": { - "debug_log": "m.setvar\\(\\) with one argument returned: nil", + "debug_log": "m.setvar: Failed m.setvar function must have 2 arguments.*m.setvar\\(\\) with one argument returned: nil", "http_code": 200 }, "rules": [ "SecRuleEngine On", "SecRuleScript test-cases/data/setvar-error-onearg.lua \"id:1,pass\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing LUA :: m.setvar with an unknown collection returns nil", + "resource": "lua", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "My sweet little browser", + "Accept": "*/*", + "Content-Length": "0" + }, + "uri": "/whee?res=1", + "method": "GET", + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "m.setvar: Unknown collection: FOO\\..*m.setvar\\(\\) with an unknown collection returned: nil", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRuleScript test-cases/data/setvar-error-unknown-collection.lua \"id:1,pass\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing LUA :: m.setvar sets a TX variable and returns nil", + "resource": "lua", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "My sweet little browser", + "Accept": "*/*", + "Content-Length": "0" + }, + "uri": "/whee?res=1", + "method": "GET", + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "m.setvar\\(\\) with TX returned: nil.*tx.a is now: x", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRuleScript test-cases/data/setvar-tx.lua \"id:1,pass\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing LUA :: m.setvar with a non-string value fails the script", + "resource": "lua", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "My sweet little browser", + "Accept": "*/*", + "Content-Length": "0" + }, + "uri": "/whee?res=1", + "method": "GET", + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Failed to execute lua script: .*setvar-error-table.lua \\(main\\) - .*bad argument #2 to 'setvar'", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRuleScript test-cases/data/setvar-error-table.lua \"id:1,pass\"" + ] } ]