CVE-2021-46871: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: tag.ex in Phoenix Phoenix.HTML (aka phoenix_html) before 3.0.4 allows XSS in HEEx class attributes.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2023-01-10T06:15Z
Repository: https://github.com/phoenixframework/phoenix_html
Language: Other
Files changed: lib/phoenix_html/tag.ex, test/phoenix_html/tag_test.exs
Commit date: 2021-09-23 09:22:43 +0200
Commit message: Make sure class={@value} in HEEx is escaped

Diff: diff --git a/lib/phoenix_html/tag.ex b/lib/phoenix_html/tag.ex
index a5aeda8..6bab12e 100644
--- a/lib/phoenix_html/tag.ex
+++ b/lib/phoenix_html/tag.ex
@@ -125,10 +125,19 @@ defmodule Phoenix.HTML.Tag do
 
   def attributes_escape(attrs) do
     {:safe, attrs |> Enum.to_list() |> build_attrs()}
   end
 
+  defp build_attrs([{k, true} | t]),
+    do: [?\s, key_escape(k) | build_attrs(t)]
+
+  defp build_attrs([{_, false} | t]),
+    do: build_attrs(t)
+
+  defp build_attrs([{_, nil} | t]),
+    do: build_attrs(t)
+
   defp build_attrs([{"data", v} | t]) when is_list(v),
     do: nested_attrs(v, " data", t)
 
   defp build_attrs([{"aria", v} | t]) when is_list(v),
     do: nested_attrs(v, " aria", t)
@@ -143,19 +152,10 @@ defmodule Phoenix.HTML.Tag do
     do: nested_attrs(v, " aria", t)
 
   defp build_attrs([{:class, v} | t]) when is_list(v),
     do: [" class=\"", class_value(v), ?" | build_attrs(t)]
 
-  defp build_attrs([{k, true} | t]),
-    do: [?\s, key_escape(k) | build_attrs(t)]
-
-  defp build_attrs([{_, false} | t]),
-    do: build_attrs(t)
-
-  defp build_attrs([{_, nil} | t]),
-    do: build_attrs(t)
-
   defp build_attrs([{k, v} | t]),
     do: [?\s, key_escape(k), ?=, ?", attr_escape(v), ?" | build_attrs(t)]
 
   defp build_attrs([]), do: []
 
@@ -170,14 +170,15 @@ defmodule Phoenix.HTML.Tag do
 
   defp class_value(value) when is_list(value) do
     value
     |> Enum.filter(& &1)
     |> Enum.join(" ")
+    |> attr_escape()
   end
 
   defp class_value(value) do
-    value
+    attr_escape(value)
   end
 
   defp key_escape(value) when is_atom(value), do: String.replace(Atom.to_string(value), "_", "-")
   defp key_escape(value), do: attr_escape(value)
 
diff --git a/test/phoenix_html/tag_test.exs b/test/phoenix_html/tag_test.exs
index 59bc29c..98918d4 100644
--- a/test/phoenix_html/tag_test.exs
+++ b/test/phoenix_html/tag_test.exs
@@ -37,15 +37,22 @@ defmodule Phoenix.HTML.TagTest do
                ~s( aria-a="1" aria-b="2")
     end
 
     test "handle class value as string" do
       assert attributes_escape([{:class, "btn"}]) |> safe_to_string() == ~s( class="btn")
+      assert attributes_escape([{:class, "<active>"}]) |> safe_to_string() == ~s( class="&lt;active&gt;")
     end
 
     test "handle class value as list" do
-      assert attributes_escape([{:class, ["btn", nil, false, "active"]}]) |> safe_to_string() ==
-               ~s( class="btn active")
+      assert attributes_escape([{:class, ["btn", nil, false, "<active>"]}]) |> safe_to_string() ==
+               ~s( class="btn &lt;active&gt;")
+    end
+
+    test "handle class value as false/nil/true" do
+      assert attributes_escape([{:class, false}]) |> safe_to_string() == ~s()
+      assert attributes_escape([{:class, nil}]) |> safe_to_string() == ~s()
+      assert attributes_escape([{:class, true}]) |> safe_to_string() == ~s( class)
     end
 
     test "handle class key as string" do
       assert attributes_escape([{"class", "btn"}]) |> safe_to_string() == ~s( class="btn")
     end

Vulnerable code:   defp build_attrs([{k, true} | t]),
    do: [?\s, key_escape(k) | build_attrs(t)]

  defp build_attrs([{_, false} | t]),
    do: build_attrs(t)

  defp build_attrs([{_, nil} | t]),
    do: build_attrs(t)

    value
      assert attributes_escape([{:class, ["btn", nil, false, "active"]}]) |> safe_to_string() ==
               ~s( class="btn active")
Fixed code:   defp build_attrs([{k, true} | t]),
    do: [?\s, key_escape(k) | build_attrs(t)]

  defp build_attrs([{_, false} | t]),
    do: build_attrs(t)

  defp build_attrs([{_, nil} | t]),
    do: build_attrs(t)

    |> attr_escape()
    attr_escape(value)
      assert attributes_escape([{:class, "<active>"}]) |> safe_to_string() == ~s( class="&lt;active&gt;")
      assert attributes_escape([{:class, ["btn", nil, false, "<active>"]}]) |> safe_to_string() ==
               ~s( class="btn &lt;active&gt;")
    end

    test "handle class value as false/nil/true" do
      assert attributes_escape([{:class, false}]) |> safe_to_string() == ~s()
      assert attributes_escape([{:class, nil}]) |> safe_to_string() == ~s()
      assert attributes_escape([{:class, true}]) |> safe_to_string() == ~s( class)