diff --git a/apisix/plugins/body-transformer.lua b/apisix/plugins/body-transformer.lua index 9cb881a30386..9570a067ff3e 100644 --- a/apisix/plugins/body-transformer.lua +++ b/apisix/plugins/body-transformer.lua @@ -121,7 +121,10 @@ local decoders = { return req_get_uri_args() end, multipart = function (data, content_type_header) - local res = multipart(data, content_type_header) + local ok, res = pcall(multipart, data, content_type_header) + if not ok then + return nil, res + end return res end } @@ -144,15 +147,15 @@ local function transform(conf, body, typ, ctx, request_method) local err if format then out, err = decoders[format](body, ct) - if format == "multipart" then - _multipart = out - out = out:get_all_with_arrays() - end if not out then err = str_format("%s body decode: %s", typ, err) - core.log.error(err, ", body=", body) + core.log.error(err, ", body size: ", body and #body or 0) return nil, 400, err end + if format == "multipart" then + _multipart = out + out = out:get_all_with_arrays() + end else core.log.warn("no input format to parse ", typ, " body") end @@ -170,6 +173,16 @@ local function transform(conf, body, typ, ctx, request_method) return nil, 503, err end + -- The helpers below are provided via __index, but Lua reads raw keys + -- before consulting __index. Clear the reserved names from the decoded + -- body so attacker-controlled fields cannot shadow the helpers and + -- break (or hijack) template rendering. + out._ctx = nil + out._body = nil + out._escape_xml = nil + out._escape_json = nil + out._multipart = nil + setmetatable(out, {__index = { _ctx = ctx, _body = body, @@ -178,7 +191,8 @@ local function transform(conf, body, typ, ctx, request_method) _multipart = _multipart }}) - local ok, render_out = pcall(render, out) + local render_out + ok, render_out = pcall(render, out) if not ok then local err = str_format("%s template rendering: %s", typ, render_out) core.log.error(err) diff --git a/apisix/plugins/cors.lua b/apisix/plugins/cors.lua index deae034e5f3f..54a786d68b8a 100644 --- a/apisix/plugins/cors.lua +++ b/apisix/plugins/cors.lua @@ -280,6 +280,10 @@ end local function process_with_allow_origins_by_regex(allow_origin_type, allow_origins_by_regex, conf, ctx, req_origin) + if not req_origin then + return + end + local allow_origins_by_regex_rules_concat_conf_key = "allow_origins_by_regex_rules_concat_" .. allow_origin_type diff --git a/apisix/plugins/multi-auth.lua b/apisix/plugins/multi-auth.lua index 7d34ffb23c41..3b9660d7b4e0 100644 --- a/apisix/plugins/multi-auth.lua +++ b/apisix/plugins/multi-auth.lua @@ -88,7 +88,7 @@ function _M.rewrite(conf, ctx) else core.table.insert(errors, auth_plugin_name .. " failed to authenticate the request, code: " - .. auth_code .. ". error: " .. err) + .. auth_code .. ". error: " .. (err or "")) end end end diff --git a/t/plugin/body-transformer.t b/t/plugin/body-transformer.t index b6a266c47a82..c301eadada4a 100644 --- a/t/plugin/body-transformer.t +++ b/t/plugin/body-transformer.t @@ -1127,3 +1127,148 @@ location /demo { } --- no_error_log no input format to parse + + + +=== TEST 17: malformed multipart body is handled gracefully (no 500) +--- config + location /demo { + content_by_lua_block { + ngx.say("should not reach upstream") + } + } + location /t { + content_by_lua_block { + local t = require("lib.test_admin") + local core = require("apisix.core") + local req_template = [[{"foo":"{{foo}}"}]] + + local code, body = t.test('/apisix/admin/routes/1', + ngx.HTTP_PUT, + string.format([[{ + "uri": "/foobar", + "plugins": { + "proxy-rewrite": { + "uri": "/demo" + }, + "body-transformer": { + "request": { + "input_format": "multipart", + "template": "%s" + } + } + }, + "upstream": { + "type": "roundrobin", + "nodes": { + "127.0.0.1:%d": 1 + } + } + }]], req_template:gsub('"', '\\"'), ngx.var.server_port) + ) + + if code >= 300 then + ngx.status = code + return + end + ngx.sleep(0.5) + + local http = require("resty.http") + local httpc = http.new() + local res = httpc:request_uri("http://127.0.0.1:" .. ngx.var.server_port .. "/foobar", { + method = "POST", + body = "this is not a valid multipart body", + headers = { + ["Content-Type"] = "multipart/form-data; boundary=----WrongBoundary", + }, + }) + -- the worker must not crash on malformed multipart input. Depending + -- on the multipart parser, a malformed body either decodes to an + -- empty part set (request proceeds) or fails decoding (400); both are + -- graceful. The regression we guard against is a 500. + ngx.say(res.status == 500 and "crashed" or "ok") + } + } +--- response_body +ok +--- no_error_log +[error] + + + +=== TEST 18: body fields cannot shadow reserved template helpers (_escape_json etc.) +--- config + location /demo { + content_by_lua_block { + local core = require("apisix.core") + local body = core.request.get_body() + local data = core.json.decode(body) + if data == nil or data.foobar ~= "safe" then + return ngx.exit(400) + end + } + } + location /t { + content_by_lua_block { + local t = require("lib.test_admin") + local core = require("apisix.core") + local req_template = [[{"foobar":{*_escape_json(name)*}}]] + local admin_body = [[{ + "uri": "/foobar", + "plugins": { + "proxy-rewrite": { + "uri": "/demo", + "method": "POST" + }, + "body-transformer": { + "request": { + "input_format": "json", + "template": "%s" + } + } + }, + "upstream": { + "type": "roundrobin", + "nodes": { + "127.0.0.1:%d": 1 + } + } + }]] + + local code, body = t.test('/apisix/admin/routes/1', + ngx.HTTP_PUT, + string.format(admin_body, req_template:gsub('"', '\\"'), ngx.var.server_port) + ) + + if code >= 300 then + ngx.status = code + return + end + ngx.sleep(0.5) + + local http = require("resty.http") + -- the body tries to shadow every reserved helper with a plain string; + -- before the fix, calling _escape_json(name) hit the string from the + -- body (raw keys win over __index) and rendering failed with 503. + local body = [[{ + "name": "safe", + "_ctx": "evil", + "_body": "evil", + "_escape_xml": "evil", + "_escape_json": "evil", + "_multipart": "evil" + }]] + local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/foobar" + local opt = {method = "POST", body = body} + local httpc = http.new() + local res = httpc:request_uri(uri, opt) + assert(res.status == 200, "expected 200, got " .. res.status) + ngx.say("ok") + } + } +--- request +GET /t +--- response_body +ok +--- no_error_log +[error] diff --git a/t/plugin/cors.t b/t/plugin/cors.t index 79e32513d98e..af0741954b8b 100644 --- a/t/plugin/cors.t +++ b/t/plugin/cors.t @@ -927,3 +927,57 @@ Access-Control-Allow-Headers: headr1,headr2 Access-Control-Expose-Headers: ex-headr1,ex-headr2 Access-Control-Max-Age: 50 Access-Control-Allow-Credentials: true + + + +=== TEST 36: set route (regex specified) +--- 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, + [[{ + "plugins": { + "cors": { + "allow_origins": "http://sub.domain.com", + "allow_methods": "GET,POST", + "allow_headers": "headr1,headr2", + "expose_headers": "ex-headr1,ex-headr2", + "allow_credential": true, + "allow_origins_by_regex":[".*\\.test.com$"] + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed + + + +=== TEST 37: request without Origin header does not error out +--- request +GET /hello HTTP/1.1 +--- response_body +hello world +--- response_headers +Access-Control-Allow-Origin: +--- error_code: 200 +--- no_error_log +[error] diff --git a/t/plugin/multi-auth.t b/t/plugin/multi-auth.t index 2bb3babb8a26..4cb017fe7139 100644 --- a/t/plugin/multi-auth.t +++ b/t/plugin/multi-auth.t @@ -611,3 +611,47 @@ hello world GET /t --- response_body hello world + + + +=== TEST 22: auth plugin returning a code with nil error message does not raise +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugin") + local orig_get = plugin.get + plugin.get = function(name) + return { + type = "auth", + rewrite = function() + -- mimic an auth plugin that returns a status code + -- without an accompanying error message + return 401, nil + end, + } + end + + local multi_auth = require("apisix.plugins.multi-auth") + local conf = { + auth_plugins = { + { ["fake-auth-1"] = {} }, + { ["fake-auth-2"] = {} }, + } + } + local ctx = { var = {} } + local ok, code = pcall(multi_auth.rewrite, conf, ctx) + plugin.get = orig_get + + if not ok then + ngx.say("error: ", code) + return + end + ngx.say("code: ", code) + } + } +--- request +GET /t +--- response_body +code: 401 +--- no_error_log +[error]