From 6d959959df9a4b8bf3108ac51594c7c71bf641f2 Mon Sep 17 00:00:00 2001 From: Neil Berkman Date: Fri, 23 Oct 2026 15:35:57 -0700 Subject: [PATCH] Count multipart preamble bytes against the parser length limit Preamble bytes before the first boundary delimiter were never charged to :length. :plug_multipart.skip_preamble/2 discards each chunk down to its last three bytes before returning {:more, rest}, so read_part_headers_more/7 only ever compared the retained bytes against the limit, and Plug.Parsers.MULTIPART subtracted only the header size from its budget. A 9 MB preamble ahead of a 2-byte part passed the default 8 MB limit on both Bandit and Cowboy. read_part_headers/8 now carries the number of discarded preamble bytes, returns {:error, :too_large, conn} as soon as they exceed the limit, and records the total in conn.private[:plug_multipart_skipped] so the parser subtracts it from its remaining budget. --- lib/plug/conn.ex | 44 +++++++++++++++++++++++++++-------- lib/plug/parsers/multipart.ex | 2 +- test/plug/parsers_test.exs | 20 ++++++++++++++++ 3 files changed, 55 insertions(+), 11 deletions(-) diff --git a/lib/plug/conn.ex b/lib/plug/conn.ex index 98dec7643..ea8e05b1c 100644 --- a/lib/plug/conn.ex +++ b/lib/plug/conn.ex @@ -1229,32 +1229,56 @@ defmodule Plug.Conn do opts = opts ++ [length: 64_000, read_length: 64_000, read_timeout: 5000] length = Keyword.fetch!(opts, :length) {data, state} = read_multipart_from_buffer_or_adapter(buffer, adapter, state, opts) - read_part_headers(conn, data, length, boundary, adapter, state, opts) + read_part_headers(conn, data, length, boundary, adapter, state, opts, 0) :done -> {:done, conn} end end - defp read_part_headers(conn, data, length, boundary, adapter, state, opts) do + # `skipped` counts preamble bytes already discarded for this part, so the + # length limit bounds everything consumed before the headers, not just the + # last chunk. + defp read_part_headers(conn, data, length, boundary, adapter, state, opts, skipped) do case :plug_multipart.parse_headers(data, boundary) do - {:ok, _headers, rest} when byte_size(data) - byte_size(rest) > length -> + {:ok, _headers, rest} when skipped + byte_size(data) - byte_size(rest) > length -> {:error, :too_large, store_multipart(conn, {boundary, rest}, adapter, state)} {:ok, headers, rest} -> - {:ok, headers, store_multipart(conn, {boundary, rest}, adapter, state)} + skipped = skipped + preamble_size(data, boundary) + conn = store_multipart(conn, {boundary, rest}, adapter, state) + {:ok, headers, put_in(conn.private[:plug_multipart_skipped], skipped)} :more -> - read_part_headers_more(conn, data, length, boundary, adapter, state, opts) + read_part_headers_more(conn, data, length, boundary, adapter, state, opts, skipped) {:more, rest} -> - read_part_headers_more(conn, rest, length, boundary, adapter, state, opts) + skipped = skipped + byte_size(data) - byte_size(rest) + + if skipped > length do + {:error, :too_large, store_multipart(conn, {boundary, rest}, adapter, state)} + else + read_part_headers_more(conn, rest, length, boundary, adapter, state, opts, skipped) + end {:done, _} -> {:done, store_multipart(conn, :done, adapter, state)} end end + defp preamble_size(data, boundary) do + prefix = "--" <> boundary + + if String.starts_with?(data, prefix) do + 0 + else + case :binary.match(data, "\r\n" <> prefix) do + {pos, _} -> pos + 2 + :nomatch -> 0 + end + end + end + @doc """ Reads the body of a multipart request. @@ -1352,14 +1376,14 @@ defmodule Plug.Conn do %{put_in(conn.private[:plug_multipart], multipart) | adapter: {adapter, state}} end - defp read_part_headers_more(conn, data, length, boundary, adapter, state, _opts) - when byte_size(data) >= length do + defp read_part_headers_more(conn, data, length, boundary, adapter, state, _opts, skipped) + when skipped + byte_size(data) >= length do {:error, :too_large, store_multipart(conn, {boundary, data}, adapter, state)} end - defp read_part_headers_more(conn, data, length, boundary, adapter, state, opts) do + defp read_part_headers_more(conn, data, length, boundary, adapter, state, opts, skipped) do {_, next, state} = next_multipart(adapter, state, opts) - read_part_headers(conn, data <> next, length, boundary, adapter, state, opts) + read_part_headers(conn, data <> next, length, boundary, adapter, state, opts, skipped) end defp read_multipart_from_buffer_or_adapter("", adapter, state, opts) do diff --git a/lib/plug/parsers/multipart.ex b/lib/plug/parsers/multipart.ex index 3653abacc..187cd101b 100644 --- a/lib/plug/parsers/multipart.ex +++ b/lib/plug/parsers/multipart.ex @@ -180,7 +180,7 @@ defmodule Plug.Parsers.MULTIPART do end defp parse_multipart_headers(headers, conn, limit, opts, acc) do - limit = limit - headers_size(headers) + limit = limit - headers_size(headers) - Map.get(conn.private, :plug_multipart_skipped, 0) case multipart_type(headers) do {:binary, name} -> diff --git a/test/plug/parsers_test.exs b/test/plug/parsers_test.exs index 62c8442cd..86a9a09c1 100644 --- a/test/plug/parsers_test.exs +++ b/test/plug/parsers_test.exs @@ -418,6 +418,26 @@ defmodule Plug.ParsersTest do assert Plug.Exception.status(exception) == 413 end + test "raises when multipart preamble plus body exceeds the parser length" do + multipart = + [ + String.duplicate("P", 800_000), + "\r\n--deadbeef\r\ncontent-disposition: form-data; name=\"x\"\r\n\r\n", + String.duplicate("a", 7_500_000), + "\r\n--deadbeef--\r\n" + ] + |> IO.iodata_to_binary() + + exception = + assert_raise Plug.Parsers.RequestTooLargeError, ~r/the request is too large/, fn -> + conn(:post, "/", multipart) + |> put_req_header("content-type", "multipart/form-data; boundary=deadbeef") + |> parse() + end + + assert Plug.Exception.status(exception) == 413 + end + test "raises when cumulative multipart headers exceed the parser length" do multipart = [