Improve validation and hardening of user-supplied data

.github/workflows/test.yml

@@ -7,9 +7,11 @@ jobs:
     strategy:
       matrix:
         include:
+          - elixir: 1.20.x
+            otp: 29.x
+            check_formatted: true
           - elixir: 1.19.x
             otp: 28.x
-            check_formatted: true
           - elixir: 1.18.x
             otp: 27.x
           - elixir: 1.17.x

.tool-versions

@@ -1,2 +1,2 @@
-elixir 1.19.5-otp-28
-erlang 28.3
+elixir 1.20.0-otp-29
+erlang 29.0.1

AGENTS.md

@@ -0,0 +1,3 @@
+## Overview
+
+`multipart` is an Elixir library that constructs multipart messages (HTTP form-data requests, multipart email) following RFC 2046 and RFC 7578.

CLAUDE.md

@@ -0,0 +1 @@
+@AGENTS.md

lib/multipart.ex

@@ -17,6 +17,7 @@ defmodule Multipart do
 
   @crlf "\r\n"
   @separator "--"
+  @boundary_regex ~r/\A[0-9A-Za-z'()+_,\-.\/:=?]{1,70}\z/
 
   alias Multipart.Part
 
@@ -26,12 +27,28 @@ defmodule Multipart do
   Pass in the boundary as the first argument to set it explicitly, otherwise
   it will default to a random 16 character alphanumeric string padded by `==`
   on either side.
+
+  The boundary must conform to the RFC 2046 boundary grammar (1-70 of the
+  allowed `bcharsnospace` characters); anything else raises `ArgumentError`.
+  The default boundary is a 128-bit random token, so it is safe against a part
+  body that happens to contain the boundary. If you supply a custom boundary
+  you are responsible for ensuring it cannot appear in any part body.
   """
   @spec new(String.t()) :: t()
   def new(boundary \\ generate_boundary()) do
+    validate_boundary!(boundary)
     %__MODULE__{boundary: boundary}
   end
 
+  # RFC 2046 boundary grammar: 1*70 bcharsnospace. Rejecting anything outside
+  # this charset prevents CRLF/quote injection into the body delimiters and the
+  # `content_type/3` header.
+  defp validate_boundary!(boundary) do
+    unless is_binary(boundary) and Regex.match?(@boundary_regex, boundary) do
+      raise ArgumentError, "invalid multipart boundary: #{inspect(boundary)}"
+    end
+  end
+
   @doc """
   Adds a part to the `Multipart` message.
   """
@@ -148,11 +165,18 @@ defmodule Multipart do
   defp part_headers(%Part{headers: headers}) do
     headers
     |> Enum.flat_map(fn {k, v} ->
+      validate_header!(k, v)
       ["#{k}: #{v}", @crlf]
     end)
     |> List.insert_at(-1, @crlf)
   end
 
+  defp validate_header!(k, v) do
+    if String.contains?("#{k}", ["\r", "\n"]) or String.contains?("#{v}", ["\r", "\n"]) do
+      raise ArgumentError, "header name/value contains CR/LF: #{inspect({k, v})}"
+    end
+  end
+
   defp part_body_stream(%Part{body: body}) when is_binary(body) do
     [body]
   end

lib/multipart/part.ex

@@ -128,12 +128,26 @@ defmodule Multipart.Part do
   defp content_disposition(type, directives) do
     directives
     |> Enum.map(fn {k, v} ->
-      "#{k}=\"#{v}\""
+      "#{k}=\"#{escape_directive_value(v)}\""
     end)
     |> List.insert_at(0, type)
     |> Enum.join("; ")
   end
 
+  # Percent-encode the characters that would otherwise let a directive value
+  # break out of its quotes or inject a new header (RFC 7578 §5.1, matching
+  # WHATWG/browser behaviour). The backslash is also encoded because RFC
+  # 2045/2183 (MIME/email) quoted-string parsers treat `\"` as an escaped
+  # quote, so a trailing backslash could otherwise escape the closing quote.
+  defp escape_directive_value(value) do
+    value
+    |> to_string()
+    |> String.replace("\\", "%5C")
+    |> String.replace("\r", "%0D")
+    |> String.replace("\n", "%0A")
+    |> String.replace("\"", "%22")
+  end
+
   defp maybe_add_content_disposition_header(headers, name) do
     unless headers
            |> Enum.map(fn {k, _} -> String.downcase(k) end)

mix.lock

@@ -1,11 +1,11 @@
 %{
   "dialyxir": {:hex, :dialyxir, "1.4.7", "dda948fcee52962e4b6c5b4b16b2d8fa7d50d8645bbae8b8685c3f9ecb7f5f4d", [:mix], [{:erlex, ">= 0.2.8", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "b34527202e6eb8cee198efec110996c25c5898f43a4094df157f8d28f27d9efe"},
   "earmark_parser": {:hex, :earmark_parser, "1.4.44", "f20830dd6b5c77afe2b063777ddbbff09f9759396500cdbe7523efd58d7a339c", [:mix], [], "hexpm", "4778ac752b4701a5599215f7030989c989ffdc4f6df457c5f36938cc2d2a2750"},
-  "erlex": {:hex, :erlex, "0.2.8", "cd8116f20f3c0afe376d1e8d1f0ae2452337729f68be016ea544a72f767d9c12", [:mix], [], "hexpm", "9d66ff9fedf69e49dc3fd12831e12a8a37b76f8651dd21cd45fcf5561a8a7590"},
-  "ex_doc": {:hex, :ex_doc, "0.39.3", "519c6bc7e84a2918b737aec7ef48b96aa4698342927d080437f61395d361dcee", [:mix], [{:earmark_parser, "~> 1.4.44", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "0590955cf7ad3b625780ee1c1ea627c28a78948c6c0a9b0322bd976a079996e1"},
+  "erlex": {:hex, :erlex, "0.2.9", "7debbbaa9f4f368b8cd648983e0f1d7963028508e9c59e9d4ed504e94ef52a55", [:mix], [], "hexpm", "8cfffc0ec7159e6d73de2ab28a588064de80f88b2798d5cbe4482cbbc200178b"},
+  "ex_doc": {:hex, :ex_doc, "0.40.3", "4a972ffe64bc07dc605af487e98fc19b72a4185f55ca031b94c0552d6071c1d9", [:mix], [{:earmark_parser, "~> 1.4.44", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "2756e357742fecd9749b489b85d67c9ce99c465f2e75728d9e6dc8d704b973de"},
   "makeup": {:hex, :makeup, "1.2.1", "e90ac1c65589ef354378def3ba19d401e739ee7ee06fb47f94c687016e3713d1", [:mix], [{:nimble_parsec, "~> 1.4", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "d36484867b0bae0fea568d10131197a4c2e47056a6fbe84922bf6ba71c8d17ce"},
   "makeup_elixir": {:hex, :makeup_elixir, "1.0.1", "e928a4f984e795e41e3abd27bfc09f51db16ab8ba1aebdba2b3a575437efafc2", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "7284900d412a3e5cfd97fdaed4f5ed389b8f2b4cb49efc0eb3bd10e2febf9507"},
-  "makeup_erlang": {:hex, :makeup_erlang, "1.0.3", "4252d5d4098da7415c390e847c814bad3764c94a814a0b4245176215615e1035", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "953297c02582a33411ac6208f2c6e55f0e870df7f80da724ed613f10e6706afd"},
+  "makeup_erlang": {:hex, :makeup_erlang, "1.1.0", "835f7e60792e08824cda445639555d7bf1bbbddb1b60b306e33cb6f6db24dc74", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "1cd6780fb1dd1a03979abaed0fe82712b0625118fd5257d3ebbf73f960c73c3c"},
   "mime": {:hex, :mime, "2.0.7", "b8d739037be7cd402aee1ba0306edfdef982687ee7e9859bee6198c1e7e2f128", [:mix], [], "hexpm", "6171188e399ee16023ffc5b76ce445eb6d9672e2e241d2df6050f3c771e80ccd"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.4.2", "8efba0122db06df95bfaa78f791344a89352ba04baedd3849593bfce4d0dc1c6", [:mix], [], "hexpm", "4b21398942dda052b403bbe1da991ccd03a053668d147d53fb8c4e0efe09c973"},
 }

test/multipart_injection_test.exs

@@ -0,0 +1,96 @@
+defmodule MultipartInjectionTest do
+  use ExUnit.Case, async: true
+
+  alias Multipart.Part
+
+  @boundary "==testboundary=="
+
+  describe "content-disposition directive injection" do
+    test "a quote in filename cannot break out to inject a directive" do
+      multipart =
+        Multipart.new(@boundary)
+        |> Multipart.add_part(
+          Part.file_content_field("/tmp/x.txt", "data", "f", [],
+            content_type: false,
+            filename: ~s(a"; name="evil)
+          )
+        )
+
+      output = Multipart.body_binary(multipart)
+
+      # The quote must be percent-encoded, not passed through verbatim.
+      assert output =~ "%22"
+      # No injected second `name=` directive may appear.
+      refute output =~ ~s(name="evil")
+    end
+
+    test "CRLF in a field name cannot inject a new header" do
+      multipart =
+        Multipart.new(@boundary)
+        |> Multipart.add_part(Part.text_field("v", "x\r\nX-Injected: yes"))
+
+      output = Multipart.body_binary(multipart)
+
+      assert output =~ "%0D%0A"
+      # The CRLF is encoded, so "X-Injected" can only survive inside the quoted
+      # value, never as its own header line (preceded by a real CRLF).
+      refute output =~ "\r\nX-Injected: yes"
+    end
+
+    test "a trailing backslash cannot escape the structural closing quote" do
+      # In RFC 2045/2183 (MIME/email) quoted-string semantics, a `\"` is an
+      # escaped quote, so a value ending in a backslash would escape the
+      # library's own closing quote and absorb the following directive.
+      multipart =
+        Multipart.new(@boundary)
+        |> Multipart.add_part(
+          Part.file_content_field("/tmp/x.txt", "data", "evil\\", [],
+            content_type: false,
+            filename: "secret.txt"
+          )
+        )
+
+      output = Multipart.body_binary(multipart)
+
+      # The backslash must be percent-encoded, not left to escape the quote.
+      assert output =~ "%5C"
+      # The name value must terminate cleanly so the filename stays a distinct
+      # directive: no `\"` (backslash immediately before a structural quote).
+      refute output =~ "\\\""
+      assert output =~ ~s(filename="secret.txt")
+    end
+  end
+
+  describe "boundary validation" do
+    test "CRLF in a custom boundary is rejected" do
+      assert_raise ArgumentError, fn -> Multipart.new("a\r\nb") end
+    end
+
+    test "a quote in a custom boundary is rejected" do
+      assert_raise ArgumentError, fn -> Multipart.new(~s(a"b)) end
+    end
+
+    test "conforming boundaries are still accepted" do
+      assert %Multipart{} = Multipart.new("==testboundary==")
+      assert %Multipart{} = Multipart.new("myboundary")
+    end
+  end
+
+  describe "generic header CRLF rejection" do
+    test "CRLF in a header value raises when serialized" do
+      multipart =
+        Multipart.new(@boundary)
+        |> Multipart.add_part(Part.binary_body("body", [{"x-custom", "a\r\nX-Injected: yes"}]))
+
+      assert_raise ArgumentError, fn -> Multipart.body_binary(multipart) end
+    end
+
+    test "CRLF in a header name raises when serialized" do
+      multipart =
+        Multipart.new(@boundary)
+        |> Multipart.add_part(Part.binary_body("body", [{"x-custom\r\nX-Injected", "yes"}]))
+
+      assert_raise ArgumentError, fn -> Multipart.body_binary(multipart) end
+    end
+  end
+end