Harden numeric character reference parsing

lib/saxy/parse_error.ex

@@ -8,6 +8,7 @@ defmodule Saxy.ParseError do
           | {:wrong_closing_tag, open_tag :: String.t(), close_tag :: String.t()}
           | {:invalid_pi, pi_name :: String.t()}
           | {:invalid_encoding, encoding :: String.t()}
+          | {:invalid_char_ref, codepoint :: non_neg_integer() | :too_long}
           | {:bad_return, {event :: atom(), return :: term()}}
 
   @type t() :: %__MODULE__{
@@ -51,6 +52,14 @@ defmodule Saxy.ParseError do
     "unexpected encoding declaration #{inspect(encoding)}, only UTF-8 is supported"
   end
 
+  defp format_message({:invalid_char_ref, :too_long}, _, _) do
+    "unexpected character reference, too many digits"
+  end
+
+  defp format_message({:invalid_char_ref, codepoint}, _, _) do
+    "unexpected character reference to code point #{codepoint}, which is not a valid XML character"
+  end
+
   defp format_message({:bad_return, {event, return}}, _, _) do
     "unexpected return #{inspect(return)} in #{inspect(event)} event handler"
   end

lib/saxy/parser/builder.ex

@@ -795,9 +795,19 @@ defmodule Saxy.Parser.Builder do
             att_value_char_dec_ref(rest, more?, original, pos, state, attributes, open_quote, att_name, acc, len + 1)
 
           ";" <> rest ->
-            codepoint = original |> binary_part(pos, len) |> String.to_integer(10)
-            pos = pos + len + 1
-            att_value(rest, more?, original, pos, state, attributes, open_quote, att_name, [acc | <<codepoint::utf8>>], 0)
+            if len == 0 do
+              Utils.parse_error(original, pos, state, {:token, :char_ref})
+            else
+              case Utils.parse_char_ref(binary_part(original, pos, len), 10) do
+                {:ok, codepoint} ->
+                  pos = pos + len + 1
+
+                  att_value(rest, more?, original, pos, state, attributes, open_quote, att_name, [acc | <<codepoint::utf8>>], 0)
+
+                {:error, reason} ->
+                  Utils.parse_error(original, pos, state, reason)
+              end
+            end
 
           _ in [""] when more? ->
             halt!(att_value_char_dec_ref("", more?, original, pos, state, attributes, open_quote, att_name, acc, len))
@@ -813,10 +823,19 @@ defmodule Saxy.Parser.Builder do
             att_value_char_hex_ref(rest, more?, original, pos, state, attributes, open_quote, att_name, acc, len + 1)
 
           ";" <> rest ->
-            codepoint = original |> binary_part(pos, len) |> String.to_integer(16)
-            pos = pos + len + 1
+            if len == 0 do
+              Utils.parse_error(original, pos, state, {:token, :char_ref})
+            else
+              case Utils.parse_char_ref(binary_part(original, pos, len), 16) do
+                {:ok, codepoint} ->
+                  pos = pos + len + 1
+
+                  att_value(rest, more?, original, pos, state, attributes, open_quote, att_name, [acc | <<codepoint::utf8>>], 0)
 
-            att_value(rest, more?, original, pos, state, attributes, open_quote, att_name, [acc | <<codepoint::utf8>>], 0)
+                {:error, reason} ->
+                  Utils.parse_error(original, pos, state, reason)
+              end
+            end
 
           _ in [""] when more? ->
             halt!(att_value_char_hex_ref("", more?, original, pos, state, attributes, open_quote, att_name, acc, len))
@@ -1051,9 +1070,13 @@ defmodule Saxy.Parser.Builder do
             if len == 0 do
               Utils.parse_error(original, pos, state, {:token, :char_ref})
             else
-              char = original |> binary_part(pos, len) |> String.to_integer(10)
+              case Utils.parse_char_ref(binary_part(original, pos, len), 10) do
+                {:ok, char} ->
+                  chardata(rest, more?, original, pos + len + 1, state, [acc | <<char::utf8>>], 0)
 
-              chardata(rest, more?, original, pos + len + 1, state, [acc | <<char::utf8>>], 0)
+                {:error, reason} ->
+                  Utils.parse_error(original, pos, state, reason)
+              end
             end
 
           char <> rest when char in ?0..?9 ->
@@ -1073,9 +1096,13 @@ defmodule Saxy.Parser.Builder do
             if len == 0 do
               Utils.parse_error(original, pos, state, [])
             else
-              char = original |> binary_part(pos, len) |> String.to_integer(16)
+              case Utils.parse_char_ref(binary_part(original, pos, len), 16) do
+                {:ok, char} ->
+                  chardata(rest, more?, original, pos + len + 1, state, [acc | <<char::utf8>>], 0)
 
-              chardata(rest, more?, original, pos + len + 1, state, [acc | <<char::utf8>>], 0)
+                {:error, reason} ->
+                  Utils.parse_error(original, pos, state, reason)
+              end
             end
 
           char <> rest when char in ?0..?9 or char in ?A..?F or char in ?a..?f ->

lib/saxy/parser/utils.ex

@@ -32,6 +32,53 @@ defmodule Saxy.Parser.Utils do
     end
   end
 
+  # Maximum number of digits allowed in a numeric character reference. The
+  # largest valid code point is U+10FFFF, which needs 7 decimal / 6 hexadecimal
+  # digits; the extra budget tolerates a few leading zeros while still bounding
+  # the integer we hand to `String.to_integer/2`, preventing huge-bignum DoS.
+  @max_decimal_char_ref_digits 10
+  @max_hexadecimal_char_ref_digits 8
+
+  @doc """
+  Parses the digits of a numeric character reference into a code point.
+
+  `base` is `10` for `&#...;` or `16` for `&#x...;`. Returns `{:ok, codepoint}`
+  for a valid XML character, or `{:error, reason}` (a `Saxy.ParseError` reason)
+  when there are too many digits or the code point is not a valid XML `Char`.
+  """
+  def parse_char_ref(digits, base) do
+    max_digits =
+      case base do
+        10 -> @max_decimal_char_ref_digits
+        16 -> @max_hexadecimal_char_ref_digits
+      end
+
+    if byte_size(digits) > max_digits do
+      {:error, {:invalid_char_ref, :too_long}}
+    else
+      codepoint = String.to_integer(digits, base)
+
+      if valid_char_ref?(codepoint) do
+        {:ok, codepoint}
+      else
+        {:error, {:invalid_char_ref, codepoint}}
+      end
+    end
+  end
+
+  @compile {:inline, [valid_char_ref?: 1]}
+
+  # Returns `true` when the code point is a valid XML 1.0 `Char`, ordered with
+  # the common printable BMP range first. See https://www.w3.org/TR/xml/#NT-Char
+  # — this excludes NUL and other disallowed control characters, the surrogate
+  # range (U+D800–U+DFFF), U+FFFE, U+FFFF, and anything beyond U+10FFFF.
+  defp valid_char_ref?(codepoint) do
+    (codepoint >= 0x20 and codepoint <= 0xD7FF) or
+      codepoint == 0x9 or codepoint == 0xA or codepoint == 0xD or
+      (codepoint >= 0xE000 and codepoint <= 0xFFFD) or
+      (codepoint >= 0x10000 and codepoint <= 0x10FFFF)
+  end
+
   def valid_pi_name?(<<l::integer, m::integer, x::integer>>)
       when x in [?X, ?x] or m in [?M, ?m] or l in [?L, ?l],
       do: false

test/saxy/parser/element_test.exs

@@ -157,6 +157,54 @@ defmodule Saxy.Parser.ElementTest do
     assert Exception.message(error) == "unexpected byte \" \", expected token: :entity_ref"
   end
 
+  test "rejects out-of-range numeric character references in element without crashing" do
+    # Surrogate code points are not valid XML characters.
+    error = refute_parse("<foo>&#xD800;</foo>")
+    assert Exception.message(error) =~ "not a valid XML character"
+
+    # Beyond the Unicode maximum (> U+10FFFF).
+    error = refute_parse("<foo>&#x110000;</foo>")
+    assert Exception.message(error) =~ "not a valid XML character"
+
+    error = refute_parse("<foo>&#1114112;</foo>")
+    assert Exception.message(error) =~ "not a valid XML character"
+
+    # Disallowed control character (NUL).
+    error = refute_parse("<foo>&#x0;</foo>")
+    assert Exception.message(error) =~ "not a valid XML character"
+  end
+
+  test "rejects overlong numeric character references in element without crashing" do
+    error = refute_parse("<foo>&#11111111111111111111;</foo>")
+    assert Exception.message(error) =~ "character reference"
+
+    error = refute_parse("<foo>&#x1111111111111111;</foo>")
+    assert Exception.message(error) =~ "character reference"
+  end
+
+  test "rejects out-of-range numeric character references in attribute without crashing" do
+    error = refute_parse(~s(<foo val="&#xD800;" />))
+    assert Exception.message(error) =~ "not a valid XML character"
+
+    error = refute_parse(~s(<foo val="&#x110000;" />))
+    assert Exception.message(error) =~ "not a valid XML character"
+
+    error = refute_parse(~s(<foo val="&#11111111111111111111;" />))
+    assert Exception.message(error) =~ "character reference"
+  end
+
+  test "parses boundary numeric character references" do
+    events = assert_parse("<foo>&#x10FFFF;</foo>")
+    assert find_event(events, :characters, <<0x10FFFF::utf8>>)
+
+    # Leading zeros within the digit budget are still accepted.
+    events = assert_parse("<foo>&#x00041;</foo>")
+    assert find_event(events, :characters, "A")
+
+    events = assert_parse("<foo>&#x9;</foo>")
+    assert find_event(events, :characters, "\t")
+  end
+
   test "malformed misc in the end of the document" do
     error = refute_parse("<foo/>bar")
     assert Exception.message(error) == "unexpected byte \"b\", expected token: :misc"