diff --git a/apache2/msc_lua.c b/apache2/msc_lua.c index b165381bd..c03dc3084 100644 --- a/apache2/msc_lua.c +++ b/apache2/msc_lua.c @@ -175,11 +175,22 @@ static apr_array_header_t *resolve_tfns(lua_State *L, int idx, modsec_rec *msr, for(i = 1; i <= n; i++) { lua_rawgeti(L, idx, i); - name = (char *)luaL_checkstring(L, -1); + + /* Skip anything but a string, a number included, instead of + * failing the whole script with luaL_checkstring(). + */ + if (lua_type(L, -1) != LUA_TSTRING) { + msr_log(msr, 8, "SecRuleScript: Transformation name must be a string, but found %s", + lua_typename(L, lua_type(L, -1))); + lua_pop(L, 1); + continue; + } + name = (char *)lua_tostring(L, -1); /* A "none" means start over */ if (strcmp("none", name) == 0) { tfn_arr->nelts = 0; + lua_pop(L, 1); continue; } @@ -189,9 +200,14 @@ static apr_array_header_t *resolve_tfns(lua_State *L, int idx, modsec_rec *msr, } else { *(msre_tfn_metadata **)apr_array_push(tfn_arr) = tfn; } + + /* Pop the entry, otherwise a table longer than LUA_MINSTACK + * writes past the stack. name isn't used after this. + */ + lua_pop(L, 1); } - } else if (lua_isstring(L, idx)) { /* The second parameter may be a simple string? */ - name = (char *)luaL_checkstring(L, idx); + } else if (lua_type(L, idx) == LUA_TSTRING) { /* The second parameter may be a simple string? */ + name = (char *)lua_tostring(L, idx); /* A "none" means start over */ if (strcmp("none", name) == 0) { @@ -206,7 +222,7 @@ static apr_array_header_t *resolve_tfns(lua_State *L, int idx, modsec_rec *msr, } } } else { - msr_log(msr, 1, "SecRuleScript: Transformation parameter must be a transformation name or array of transformation names, but found \"%s\" (type %d).", lua_typename(L, idx), lua_type(L, idx)); + msr_log(msr, 1, "SecRuleScript: Transformation parameter must be a transformation name or array of transformation names, but found \"%s\" (type %d).", lua_typename(L, lua_type(L, idx)), lua_type(L, idx)); return NULL; } diff --git a/tests/regression/rule/00-script.t b/tests/regression/rule/00-script.t index e57bdda39..ffb4ed76d 100644 --- a/tests/regression/rule/00-script.t +++ b/tests/regression/rule/00-script.t @@ -118,3 +118,60 @@ GET => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", ), }, +{ + type => "rule", + comment => "SecRuleScript m.getvar with a long transformation table", + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRuleScript "tfn-long-table.lua" "phase:2,pass" + ), + match_log => { + debug => [ qr/long table: abc/, 1 ], + }, + match_response => { + status => qr/^200$/, + }, + request => new HTTP::Request( + GET => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt?a=ABC", + ), +}, +{ + type => "rule", + comment => "SecRuleScript m.getvar skips a transformation that isn't a string", + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRuleScript "tfn-not-a-string.lua" "phase:2,pass" + ), + match_log => { + debug => [ qr/Transformation name must be a string, but found table.*Transformation name must be a string, but found number.*not a string: abc/s, 1 ], + }, + match_response => { + status => qr/^200$/, + }, + request => new HTTP::Request( + GET => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt?a=ABC", + ), +}, +{ + type => "rule", + comment => "SecRuleScript m.getvar logs the type of a bad transformation parameter", + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRuleScript "tfn-bad-param.lua" "phase:2,pass" + ), + match_log => { + debug => [ qr/but found "boolean" \(type 1\).*bad param: ABC/s, 1 ], + }, + match_response => { + status => qr/^200$/, + }, + request => new HTTP::Request( + GET => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt?a=ABC", + ), +}, diff --git a/tests/regression/server_root/conf/tfn-bad-param.lua b/tests/regression/server_root/conf/tfn-bad-param.lua new file mode 100644 index 000000000..e070c83e9 --- /dev/null +++ b/tests/regression/server_root/conf/tfn-bad-param.lua @@ -0,0 +1,5 @@ +function main() + local v = m.getvar("ARGS.a", true); + m.log(4, "bad param: " .. tostring(v)); + return nil; +end diff --git a/tests/regression/server_root/conf/tfn-long-table.lua b/tests/regression/server_root/conf/tfn-long-table.lua new file mode 100644 index 000000000..10c8a9a56 --- /dev/null +++ b/tests/regression/server_root/conf/tfn-long-table.lua @@ -0,0 +1,7 @@ +function main() + local t = {}; + for i = 1, 1000 do t[i] = "lowercase"; end + local v = m.getvar("ARGS.a", t); + m.log(4, "long table: " .. tostring(v)); + return nil; +end diff --git a/tests/regression/server_root/conf/tfn-not-a-string.lua b/tests/regression/server_root/conf/tfn-not-a-string.lua new file mode 100644 index 000000000..5f3ad2278 --- /dev/null +++ b/tests/regression/server_root/conf/tfn-not-a-string.lua @@ -0,0 +1,5 @@ +function main() + local v = m.getvar("ARGS.a", { "lowercase", {}, 12 }); + m.log(4, "not a string: " .. tostring(v)); + return nil; +end