✨ Feature / SafeXml — single hardened xmerl_scan entry point

lib/ex_saml/core/binding.ex

@@ -33,6 +33,8 @@ defmodule ExSaml.Core.Binding do
   configured with `use_redirect_for_req: true`.
   """
 
+  alias ExSaml.Core.Xml.SafeXml
+
   require Record
 
   Record.defrecord(:xmlElement, Record.extract(:xmlElement, from_lib: "xmerl/include/xmerl.hrl"))
@@ -60,32 +62,27 @@ defmodule ExSaml.Core.Binding do
   unzip is attempted, falling back to the raw decoded data if decompression
   fails. The resulting XML string is then parsed with `:xmerl_scan`.
   """
-  @spec decode_response(binary(), binary()) :: xml()
+  @spec decode_response(binary(), binary()) :: {:ok, xml()} | {:error, :invalid_xml}
   def decode_response(@deflate, saml_response) do
-    xml_data =
-      saml_response
-      |> :base64.decode()
-      |> :zlib.unzip()
-      |> :binary.bin_to_list()
-
-    {xml, _} = :xmerl_scan.string(xml_data, namespace_conformant: true, allow_entities: false)
-    xml
+    saml_response
+    |> :base64.decode()
+    |> :zlib.unzip()
+    |> SafeXml.scan()
   end
 
   def decode_response(_encoding, saml_response) do
     data = :base64.decode(saml_response)
 
     xml_data =
       try do
-        :zlib.unzip(data) |> :binary.bin_to_list()
+        :zlib.unzip(data)
       rescue
-        _e -> :binary.bin_to_list(data)
+        _e -> data
       catch
-        _kind, _reason -> :binary.bin_to_list(data)
+        _kind, _reason -> data
       end
 
-    {xml, _} = :xmerl_scan.string(xml_data, namespace_conformant: true, allow_entities: false)
-    xml
+    SafeXml.scan(xml_data)
   end
 
   @doc """

lib/ex_saml/core/sp.ex

@@ -32,7 +32,8 @@ defmodule ExSaml.Core.Sp do
     SpMetadata,
     Subject,
     Util,
-    Xml.Dsig
+    Xml.Dsig,
+    Xml.SafeXml
   }
 
   @type xml :: record(:xmlElement)
@@ -298,18 +299,13 @@ defmodule ExSaml.Core.Sp do
            [{:namespace, ns}]
          ) do
       [encrypted] when Record.is_record(encrypted, :xmlElement) ->
-        try do
-          decrypted = decrypt_assertion(encrypted, sp)
-          true = Record.is_record(decrypted, :xmlElement)
-
-          case :xmerl_xpath.string(~c"/saml:Assertion", decrypted, [{:namespace, ns}]) do
-            [a] -> {:ok, a}
-            _ -> {:error, :bad_assertion}
-          end
-        rescue
+        with {:ok, decrypted} <- decrypt_assertion(encrypted, sp),
+             true <- Record.is_record(decrypted, :xmlElement),
+             [assertion] <-
+               :xmerl_xpath.string(~c"/saml:Assertion", decrypted, [{:namespace, ns}]) do
+          {:ok, assertion}
+        else
           _ -> {:error, :bad_assertion}
-        catch
-          _, _ -> {:error, :bad_assertion}
         end
 
       _ ->
@@ -516,13 +512,11 @@ defmodule ExSaml.Core.Sp do
 
     assertion_xml = block_decrypt(to_string(algorithm), symmetric_key, cipher_value)
 
-    {assertion, _} =
-      :xmerl_scan.string(:binary.bin_to_list(assertion_xml),
-        namespace_conformant: true,
-        allow_entities: false
-      )
-
-    assertion
+    SafeXml.scan(assertion_xml)
+  rescue
+    _ -> {:error, :decrypt_failed}
+  catch
+    _, _ -> {:error, :decrypt_failed}
   end
 
   defp decrypt_key_info(encrypted_data, key) do

lib/ex_saml/core/xml/safe_xml.ex

@@ -0,0 +1,97 @@
+defmodule ExSaml.Core.Xml.SafeXml do
+  @moduledoc """
+  Single entry point for parsing untrusted SAML/XML payloads with `:xmerl_scan`.
+
+  All calls go through this module so that the XXE-safe options
+  (`allow_entities: false`, `namespace_conformant: true`) and the
+  binary→charlist conversion are applied **once**, in one place. Callers do
+  not pass scan options — they cannot weaken the defaults.
+
+  Centralising the wrapper means a future hardening change (new xmerl option,
+  stricter limits, alternative parser) is a one-file diff. It also
+  centralises the binary→charlist conversion that previously had to be
+  patched at every call site (cf. UTF-8 SAMLResponse handling).
+
+  ## Error reporting hook
+
+  An arity-1 function may be configured to receive parse-failure context.
+  Typical use: forward to a SIEM / OCSF pipeline.
+
+      config :ex_saml, :safe_xml,
+        on_error: &MyApp.SIEM.report_xml_error/1
+
+  The handler is invoked **after** the `{:error, :invalid_xml}` tuple is
+  produced and receives a map:
+
+      %{
+        reason: :invalid_xml,
+        kind: :error | :exit | :throw,
+        payload: Exception.t() | term()
+      }
+
+  Any exception raised by the handler is silently caught — the handler must
+  never be able to mask the original parse failure or alter the caller's
+  control flow.
+  """
+
+  @scan_opts [namespace_conformant: true, allow_entities: false, quiet: true]
+
+  @type xml :: term()
+  @type error_context :: %{
+          required(:reason) => :invalid_xml,
+          required(:kind) => :error | :exit | :throw,
+          required(:payload) => Exception.t() | term()
+        }
+
+  @doc """
+  Parse `xml` with `:xmerl_scan` using the hardened option set.
+
+  Accepts either a UTF-8 binary or a charlist. Returns the parsed root on
+  success or `{:error, :invalid_xml}` on any parse failure.
+
+  Invokes the configured `:on_error` hook (if any) before returning the
+  error tuple.
+  """
+  @spec scan(binary() | charlist()) :: {:ok, xml()} | {:error, :invalid_xml}
+  def scan(xml) when is_binary(xml), do: xml |> :binary.bin_to_list() |> scan()
+
+  def scan(xml) when is_list(xml) do
+    {root, _rest} = :xmerl_scan.string(xml, @scan_opts)
+    {:ok, root}
+  rescue
+    e ->
+      notify(%{reason: :invalid_xml, kind: :error, payload: e})
+      {:error, :invalid_xml}
+  catch
+    :exit, reason ->
+      notify(%{reason: :invalid_xml, kind: :exit, payload: reason})
+      {:error, :invalid_xml}
+
+    :throw, value ->
+      notify(%{reason: :invalid_xml, kind: :throw, payload: value})
+      {:error, :invalid_xml}
+  end
+
+  # ---------------------------------------------------------------------------
+  # Error reporting hook
+  # ---------------------------------------------------------------------------
+
+  defp notify(context) do
+    case Application.get_env(:ex_saml, :safe_xml, [])[:on_error] do
+      fun when is_function(fun, 1) ->
+        safe_invoke(fun, context)
+
+      _ ->
+        :ok
+    end
+  end
+
+  defp safe_invoke(fun, context) do
+    fun.(context)
+    :ok
+  rescue
+    _ -> :ok
+  catch
+    _, _ -> :ok
+  end
+end

lib/ex_saml/helper.ex

@@ -108,9 +108,9 @@ defmodule ExSaml.Helper do
   end
 
   defp decode_saml_payload(saml_encoding, saml_payload) do
-    xml = Core.Binding.decode_response(saml_encoding, saml_payload)
-    {:ok, xml}
-  rescue
-    error -> {:error, {:invalid_response, "#{inspect(error)}"}}
+    case Core.Binding.decode_response(saml_encoding, saml_payload) do
+      {:ok, xml} -> {:ok, xml}
+      {:error, reason} -> {:error, {:invalid_response, reason}}
+    end
   end
 end

lib/ex_saml/metadata.ex

@@ -32,6 +32,7 @@ defmodule ExSaml.Metadata do
   See `ExSaml.Metadata.ValidationResult` for the shape of the returned struct.
   """
 
+  alias ExSaml.Core.Xml.SafeXml
   alias ExSaml.Metadata.ValidationResult
 
   require Record
@@ -100,26 +101,16 @@ defmodule ExSaml.Metadata do
   # ---------------------------------------------------------------------------
 
   defp parse(xml) do
-    charlist = :binary.bin_to_list(xml)
-
-    try do
-      {root, _rest} =
-        :xmerl_scan.string(
-          charlist,
-          namespace_conformant: true,
-          allow_entities: false,
-          quiet: true
-        )
-
-      if Record.is_record(root, :xmlElement) do
-        {:ok, root}
-      else
+    case SafeXml.scan(xml) do
+      {:ok, root} ->
+        if Record.is_record(root, :xmlElement) do
+          {:ok, root}
+        else
+          {:error, invalid_xml_violation()}
+        end
+
+      {:error, _} ->
         {:error, invalid_xml_violation()}
-      end
-    rescue
-      _ -> {:error, invalid_xml_violation()}
-    catch
-      _kind, _reason -> {:error, invalid_xml_violation()}
     end
   end
 

test/ex_saml/core/binding_test.exs

@@ -101,7 +101,7 @@ defmodule ExSaml.Core.BindingTest do
       [_, b64_payload] = Regex.run(~r/name="SAMLRequest" value="([^"]+)"/, html)
 
       # decode_response with nil encoding (non-deflate path) should parse it
-      decoded = Binding.decode_response("", b64_payload)
+      {:ok, decoded} = Binding.decode_response("", b64_payload)
       assert Record.is_record(decoded, :xmlElement)
       assert xmlElement(decoded, :name) == :AuthnRequest
     end
@@ -170,7 +170,7 @@ defmodule ExSaml.Core.BindingTest do
       compressed = :zlib.zip(xml_str)
       b64 = Base.encode64(compressed)
 
-      decoded =
+      {:ok, decoded} =
         Binding.decode_response(
           "urn:oasis:names:tc:SAML:2.0:bindings:URL-Encoding:DEFLATE",
           b64
@@ -184,7 +184,7 @@ defmodule ExSaml.Core.BindingTest do
       xml_str = "<Response>hello</Response>"
       b64 = Base.encode64(xml_str)
 
-      decoded = Binding.decode_response("", b64)
+      {:ok, decoded} = Binding.decode_response("", b64)
 
       assert Record.is_record(decoded, :xmlElement)
       assert xmlElement(decoded, :name) == :Response
@@ -234,7 +234,7 @@ defmodule ExSaml.Core.BindingTest do
         xml = "<Response><Name>" <> sample <> "</Name></Response>"
         b64 = Base.encode64(xml)
 
-        decoded = Binding.decode_response("", b64)
+        {:ok, decoded} = Binding.decode_response("", b64)
 
         assert Record.is_record(decoded, :xmlElement)
         assert xmlElement(decoded, :name) == :Response
@@ -248,7 +248,7 @@ defmodule ExSaml.Core.BindingTest do
         xml = "<Response label=\"" <> attr <> "\"><Name>x</Name></Response>"
         b64 = Base.encode64(xml)
 
-        decoded = Binding.decode_response("", b64)
+        {:ok, decoded} = Binding.decode_response("", b64)
 
         assert Record.is_record(decoded, :xmlElement)
         assert attribute_value(decoded, :label) == unquote(expected)
@@ -261,7 +261,7 @@ defmodule ExSaml.Core.BindingTest do
         xml = "<Response><Name>" <> sample <> "</Name></Response>"
         b64 = xml |> :zlib.zip() |> Base.encode64()
 
-        decoded =
+        {:ok, decoded} =
           Binding.decode_response(
             "urn:oasis:names:tc:SAML:2.0:bindings:URL-Encoding:DEFLATE",
             b64
@@ -283,7 +283,7 @@ defmodule ExSaml.Core.BindingTest do
           "</AttributeStatement></Assertion></samlp:Response>"
 
       b64 = Base.encode64(xml)
-      decoded = Binding.decode_response("", b64)
+      {:ok, decoded} = Binding.decode_response("", b64)
 
       assert Record.is_record(decoded, :xmlElement)
       assert xmlElement(decoded, :name) == :"samlp:Response"