Respect length when decoding multipart headers

Author: josevalim

lib/plug/conn.ex

@@ -1196,8 +1196,10 @@ defmodule Plug.Conn do
   @doc """
   Reads the headers of a multipart request.
 
-  It returns `{:ok, headers, conn}` with the headers or
-  `{:done, conn}` if there are no more parts.
+  It returns `{:ok, headers, conn}` with the headers,
+  `{:error, :too_large, conn}` if the current multipart header block
+  exceeds the configured `:length`, or `{:done, conn}` if there are
+  no more parts.
 
   Once `read_part_headers/2` is invoked, you may call
   `read_part_body/2` to read the body associated to the headers.
@@ -1206,40 +1208,42 @@ defmodule Plug.Conn do
 
   ## Options
 
-    * `:length` - sets the maximum number of bytes to read from the body for
-      each chunk, defaults to `64_000` bytes
+    * `:length` - sets the maximum number of bytes to read while parsing the
+      current multipart header block, defaults to `64_000` bytes
     * `:read_length` - sets the amount of bytes to read at one time from the
       underlying socket to fill the chunk, defaults to `64_000` bytes
     * `:read_timeout` - sets the timeout for each socket read, defaults to
       `5_000` milliseconds
 
   """
-  @spec read_part_headers(t, Keyword.t()) :: {:ok, headers, t} | {:done, t}
+  @spec read_part_headers(t, Keyword.t()) ::
+          {:ok, headers, t} | {:error, :too_large, t} | {:done, t}
   def read_part_headers(%Conn{adapter: {adapter, state}} = conn, opts \\ []) do
-    opts = opts ++ [length: 64_000, read_length: 64_000, read_timeout: 5000]
-
     case init_multipart(conn) do
       {boundary, buffer} ->
+        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, boundary, adapter, state, opts)
+        read_part_headers(conn, data, length, boundary, adapter, state, opts)
 
       :done ->
         {:done, conn}
     end
   end
 
-  defp read_part_headers(conn, data, boundary, adapter, state, opts) do
+  defp read_part_headers(conn, data, length, boundary, adapter, state, opts) do
     case :plug_multipart.parse_headers(data, boundary) do
+      {:ok, _headers, rest} when byte_size(data) - byte_size(rest) > length ->
+        {:error, :too_large, store_multipart(conn, {boundary, data}, adapter, state)}
+
       {:ok, headers, rest} ->
         {:ok, headers, store_multipart(conn, {boundary, rest}, adapter, state)}
 
       :more ->
-        {_, next, state} = next_multipart(adapter, state, opts)
-        read_part_headers(conn, data <> next, boundary, adapter, state, opts)
+        read_part_headers_more(conn, data, length, boundary, adapter, state, opts)
 
       {:more, rest} ->
-        {_, next, state} = next_multipart(adapter, state, opts)
-        read_part_headers(conn, rest <> next, boundary, adapter, state, opts)
+        read_part_headers_more(conn, rest, length, boundary, adapter, state, opts)
 
       {:done, _} ->
         {:done, store_multipart(conn, :done, adapter, state)}
@@ -1328,6 +1332,16 @@ 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
+    {:error, :too_large, store_multipart(conn, {boundary, data}, adapter, state)}
+  end
+
+  defp read_part_headers_more(conn, data, length, boundary, adapter, state, opts) do
+    {_, next, state} = next_multipart(adapter, state, opts)
+    read_part_headers(conn, data <> next, length, boundary, adapter, state, opts)
+  end
+
   defp read_multipart_from_buffer_or_adapter("", adapter, state, opts) do
     {_, data, state} = adapter.read_req_body(state, opts)
     {data, state}

lib/plug/parsers/multipart.ex

@@ -22,10 +22,6 @@ defmodule Plug.Parsers.MULTIPART do
   Besides the options supported by `Plug.Conn.read_body/2`, the multipart parser
   also checks for:
 
-    * `:headers` - containing the same `:length`, `:read_length`
-      and `:read_timeout` options which are used explicitly for parsing multipart
-      headers
-
     * `:validate_utf8` - specifies whether multipart body parts should be validated
       as utf8 binaries. It is either a boolean or a custom exception to raise
 
@@ -102,15 +98,12 @@ defmodule Plug.Parsers.MULTIPART do
     {read_length, opts} = Keyword.pop(opts, :read_length, 1_000_000)
     opts = [length: read_length, read_length: read_length] ++ opts
 
-    # The header options are handled individually.
-    {headers_opts, opts} = Keyword.pop(opts, :headers, [])
-
     unless is_integer(limit) do
       raise ":length option for Plug.Parsers.MULTIPART must be an integer"
     end
 
     m2p = opts[:multipart_to_params] || {__MODULE__, :multipart_to_params, [opts]}
-    {m2p, limit, headers_opts, opts}
+    {m2p, limit, opts}
   end
 
   @impl true
@@ -120,7 +113,7 @@ defmodule Plug.Parsers.MULTIPART do
       parse_multipart(conn, opts_tuple)
     rescue
       # Do not ignore upload errors
-      e in [Plug.UploadError, Plug.Parsers.BadEncodingError] ->
+      e in [Plug.UploadError, Plug.Parsers.BadEncodingError, Plug.Parsers.RequestTooLargeError] ->
         reraise e, __STACKTRACE__
 
       # All others are wrapped
@@ -146,29 +139,43 @@ defmodule Plug.Parsers.MULTIPART do
 
   ## Multipart
 
-  defp parse_multipart(conn, {m2p, limit, headers_opts, opts}) do
-    read_result = Plug.Conn.read_part_headers(conn, headers_opts)
-    {:ok, limit, acc, conn} = parse_multipart(read_result, limit, opts, headers_opts, [])
+  defp parse_multipart(conn, {m2p, limit, opts}) do
+    read_result = read_part_headers(conn, limit, opts)
 
-    if limit > 0 do
-      {mod, fun, args} = m2p
-      apply(mod, fun, [acc, conn | args])
-    else
-      {:error, :too_large, conn}
+    case parse_multipart(read_result, limit, opts, []) do
+      {:ok, limit, acc, conn} ->
+        if limit >= 0 do
+          {mod, fun, args} = m2p
+          apply(mod, fun, [acc, conn | args])
+        else
+          {:error, :too_large, conn}
+        end
+
+      {:error, :too_large, conn} ->
+        {:error, :too_large, conn}
     end
   end
 
-  defp parse_multipart({:ok, headers, conn}, limit, opts, headers_opts, acc) when limit >= 0 do
+  defp parse_multipart({:ok, headers, conn}, limit, opts, acc) when limit >= 0 do
     {conn, limit, acc} = parse_multipart_headers(headers, conn, limit, opts, acc)
-    read_result = Plug.Conn.read_part_headers(conn, headers_opts)
-    parse_multipart(read_result, limit, opts, headers_opts, acc)
+
+    if limit >= 0 do
+      read_result = read_part_headers(conn, limit, opts)
+      parse_multipart(read_result, limit, opts, acc)
+    else
+      {:ok, limit, acc, conn}
+    end
+  end
+
+  defp parse_multipart({:error, :too_large, conn}, _limit, _opts, _acc) do
+    {:error, :too_large, conn}
   end
 
-  defp parse_multipart({:ok, _headers, conn}, limit, _opts, _headers_opts, acc) do
+  defp parse_multipart({:ok, _headers, conn}, limit, _opts, acc) do
     {:ok, limit, acc, conn}
   end
 
-  defp parse_multipart({:done, conn}, limit, _opts, _headers_opts, acc) do
+  defp parse_multipart({:done, conn}, limit, _opts, acc) do
     {:ok, limit, acc, conn}
   end
 
@@ -296,4 +303,9 @@ defmodule Plug.Parsers.MULTIPART do
       nil -> nil
     end
   end
+
+  defp read_part_headers(conn, limit, opts) do
+    headers_length = min(limit, Keyword.fetch!(opts, :length))
+    Plug.Conn.read_part_headers(conn, Keyword.put(opts, :length, headers_length))
+  end
 end

test/plug/conn_test.exs

@@ -902,6 +902,40 @@ defmodule Plug.ConnTest do
     assert {:more, _, _} = read_body(conn, length: 100)
   end
 
+  test "read_part_headers/2 returns too_large while accumulating multipart headers" do
+    body =
+      [
+        "--deadbeef\r\ncontent-disposition: form-data; name=\"x\"\r\ncontent-type: ",
+        String.duplicate("a", 2_000),
+        "\r\n\r\nabc\r\n--deadbeef--\r\n"
+      ]
+      |> IO.iodata_to_binary()
+
+    conn =
+      conn(:post, "/", body)
+      |> put_req_header("content-type", "multipart/form-data; boundary=deadbeef")
+
+    assert {:error, :too_large, _conn} = read_part_headers(conn, length: 1_000)
+  end
+
+  test "read_part_headers/2 returns too_large for buffered multipart headers over the limit" do
+    buffer =
+      [
+        "--deadbeef\r\ncontent-disposition: form-data; name=\"x\"\r\ncontent-type: ",
+        String.duplicate("a", 2_000),
+        "\r\n\r\nabc\r\n--deadbeef--\r\n"
+      ]
+      |> IO.iodata_to_binary()
+
+    conn =
+      conn(:post, "/", "")
+      |> put_req_header("content-type", "multipart/form-data; boundary=deadbeef")
+
+    conn = %{conn | private: Map.put(conn.private, :plug_multipart, {"deadbeef", buffer})}
+
+    assert {:error, :too_large, _conn} = read_part_headers(conn, length: 1_000)
+  end
+
   test "query_params/1 and fetch_query_params/1" do
     conn = conn(:get, "/foo?a=b&c=d")
     assert conn.query_params == %Plug.Conn.Unfetched{aspect: :query_params}

test/plug/parsers_test.exs

@@ -389,6 +389,25 @@ defmodule Plug.ParsersTest do
     assert Plug.Exception.status(exception) == 413
   end
 
+  test "raises on multipart headers larger than the parser length" do
+    multipart =
+      [
+        "--deadbeef\r\ncontent-disposition: form-data; name=\"x\"\r\ncontent-type: ",
+        String.duplicate("a", 2_000),
+        "\r\n\r\nabc\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(length: 1_000)
+      end
+
+    assert Plug.Exception.status(exception) == 413
+  end
+
   test "raises when request cannot be processed" do
     message = "unsupported media type text/plain"