diff --git a/src/engine/lua.cc b/src/engine/lua.cc index b256b57d68..0aca87093c 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") { @@ -403,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-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/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 1c20b35f5a..5f5047ffca 100644 --- a/test/test-cases/regression/collection-lua.json +++ b/test/test-cases/regression/collection-lua.json @@ -310,5 +310,220 @@ "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: Must specify a collection using dot character.*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: 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\"" + ] } ]