fix: harden cors, multi-auth and body-transformer plugins against malformed requests

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)

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
 

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

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]

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]

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]