Merge pull request #29 from ryanwinchester-forks/fix/xss

[Security] Fix for potential XSS vulnerability
tompave · Aug 27, 2023 · 69066a4 · 69066a4
2 parents 8956464 + 5dc2466
commit 69066a4
Show file tree

Hide file tree

Showing 11 changed files with 91 additions and 20 deletions.
diff --git a/lib/fun_with_flags/ui/templates.ex b/lib/fun_with_flags/ui/templates.ex
@@ -75,9 +75,66 @@ defmodule FunWithFlags.UI.Templates do
     Path.join(conn.assigns[:namespace], path)
   end
 
-  def html_safe(val) do
+
+  def url_safe(val) do
     val
     |> to_string()
     |> URI.encode()
   end
+
+
+  @doc """
+  Escape HTML using the same code that `Phoenix.HTML` uses.
+  See: https://github.com/phoenixframework/phoenix_html/blob/v3.3.1/lib/phoenix_html/engine.ex#L24
+  """
+  def html_safe(val) when is_binary(val) do
+    html_safe(val, 0, val, [])
+  end
+
+  def html_safe(val) do
+    val
+    |> to_string()
+    |> html_safe()
+  end
+
+  escapes = [
+    {?<, "&lt;"},
+    {?>, "&gt;"},
+    {?&, "&amp;"},
+    {?", "&quot;"},
+    {?', "&#39;"}
+  ]
+
+  for {match, insert} <- escapes do
+    defp html_safe(<<unquote(match), rest::bits>>, skip, original, acc) do
+      html_safe(rest, skip + 1, original, [acc | unquote(insert)])
+    end
+  end
+
+  defp html_safe(<<_char, rest::bits>>, skip, original, acc) do
+    html_safe(rest, skip, original, acc, 1)
+  end
+
+  defp html_safe(<<>>, _skip, _original, acc) do
+    acc
+  end
+
+  for {match, insert} <- escapes do
+    defp html_safe(<<unquote(match), rest::bits>>, skip, original, acc, len) do
+      part = binary_part(original, skip, len)
+      html_safe(rest, skip + len + 1, original, [acc, part | unquote(insert)])
+    end
+  end
+
+  defp html_safe(<<_char, rest::bits>>, skip, original, acc, len) do
+    html_safe(rest, skip, original, acc, len + 1)
+  end
+
+  defp html_safe(<<>>, 0, original, _acc, _len) do
+    original
+  end
+
+  defp html_safe(<<>>, skip, original, acc, len) do
+    [acc | binary_part(original, skip, len)]
+  end
 end
diff --git a/lib/fun_with_flags/ui/templates/details.html.eex b/lib/fun_with_flags/ui/templates/details.html.eex
@@ -15,7 +15,7 @@
           </span>
 
           <li class="nav-item active">
-            <a class="nav-link" href="<%= path(@conn, "/flags/#{html_safe(@flag.name)}") %>"><%= html_safe(@flag.name) %></a>
+            <a class="nav-link" href="<%= path(@conn, "/flags/#{url_safe(@flag.name)}") %>"><%= url_safe(@flag.name) %></a>
           </li>
         </ul>
 
@@ -130,7 +130,7 @@
               Danger Zone
             </h5>
             <div class="card-block">
-              <form id="fwf-delete-flag-form" action="<%= path(@conn, "/flags/#{html_safe(@flag.name)}") %>" method="post">
+              <form id="fwf-delete-flag-form" action="<%= path(@conn, "/flags/#{url_safe(@flag.name)}") %>" method="post">
                 <input type="hidden" name="_method" value="DELETE">
                 <input type="hidden" name="_csrf_token" value="<%= @conn.assigns[:csrf_token] %>">
                 <button type="submit" class="btn btn-danger" data-confirm="Are you sure you want to completely delete the flag '<%= html_safe(@flag.name) %>'?">Delete Flag</button>

diff --git a/lib/fun_with_flags/ui/templates/index.html.eex b/lib/fun_with_flags/ui/templates/index.html.eex
@@ -30,7 +30,7 @@
               <%= for flag <- @flags do %>
                 <tr>
                   <td>
-                    <a href="<%= path(@conn, "/flags/#{html_safe(flag.name)}") %>">
+                    <a href="<%= path(@conn, "/flags/#{url_safe(flag.name)}") %>">
                       <%= html_safe(flag.name) %>
                     </a>
                   </td>

diff --git a/lib/fun_with_flags/ui/templates/rows/_actor.html.eex b/lib/fun_with_flags/ui/templates/rows/_actor.html.eex
@@ -1,13 +1,13 @@
-<div id="actor_<%= @gate.for %>" class="container fwf-I-hate-grids">
+<div id="actor_<%= html_safe(@gate.for) %>" class="container fwf-I-hate-grids">
   <div class="row no-gutters">
     <div class="col-lg-8 col-md-7 col-sm-5 col-3 text-left">
-      <code><%= @gate.for %></code>
+      <code><%= html_safe(@gate.for) %></code>
     </div>
     <div class="col-lg-2 col-md-2 col-sm-3 col-3 text-left">
       <%= html_status_for @gate.enabled %>
     </div>
     <div class="col-lg-1 col-md-2 col-sm-2 col-3 text-right">
-      <form action="<%= path(@conn, "/flags/#{html_safe(@flag.name)}/actors/#{@gate.for}") %>" method="post" class="fwf-inline-toggle">
+      <form action="<%= path(@conn, "/flags/#{url_safe(@flag.name)}/actors/#{url_safe(@gate.for)}") %>" method="post" class="fwf-inline-toggle">
         <input type="hidden" name="_method" value="PATCH">
         <input type="hidden" name="_csrf_token" value="<%= @conn.assigns[:csrf_token] %>">
 
@@ -21,10 +21,10 @@
       </form>
     </div>
     <div class="col-lg-1 col-md-1 col-sm-2 col-3 text-right">
-      <form action="<%= path(@conn, "/flags/#{html_safe(@flag.name)}/actors/#{@gate.for}") %>" method="post" class="float-right">
+      <form action="<%= path(@conn, "/flags/#{url_safe(@flag.name)}/actors/#{url_safe(@gate.for)}") %>" method="post" class="float-right">
         <input type="hidden" name="_method" value="DELETE">
         <input type="hidden" name="_csrf_token" value="<%= @conn.assigns[:csrf_token] %>">
-        <button type="submit" class="btn btn-sm btn-secondary" data-confirm="Are you sure you want to clear actor '<%= @gate.for %>'?">Clear</button>
+        <button type="submit" class="btn btn-sm btn-secondary" data-confirm="Are you sure you want to clear actor '<%= html_safe(@gate.for) %>'?">Clear</button>
       </form>
     </div>
   </div>

diff --git a/lib/fun_with_flags/ui/templates/rows/_boolean.html.eex b/lib/fun_with_flags/ui/templates/rows/_boolean.html.eex
@@ -4,7 +4,7 @@
     {:ok, bool} -> bool
     :missing -> false
   end
-  form_path = path(@conn, "/flags/#{html_safe(@flag.name)}/boolean")
+  form_path = path(@conn, "/flags/#{url_safe(@flag.name)}/boolean")
 %>
 
 <div class="card-block">

diff --git a/lib/fun_with_flags/ui/templates/rows/_group.html.eex b/lib/fun_with_flags/ui/templates/rows/_group.html.eex
@@ -1,13 +1,13 @@
-<div id="group_<%= @gate.for %>" class="container fwf-I-hate-grids">
+<div id="group_<%= html_safe(@gate.for) %>" class="container fwf-I-hate-grids">
   <div class="row no-gutters">
     <div class="col-lg-8 col-md-7 col-sm-5 col-3 text-left">
-      <code><%= @gate.for %></code>
+      <code><%= html_safe(@gate.for) %></code>
     </div>
     <div class="col-lg-2 col-md-2 col-sm-3 col-3 text-left">
       <%= html_status_for @gate.enabled %>
     </div>
     <div class="col-lg-1 col-md-2 col-sm-2 col-3 text-right">
-      <form action="<%= path(@conn, "/flags/#{html_safe(@flag.name)}/groups/#{@gate.for}") %>" method="post" class="fwf-inline-toggle">
+      <form action="<%= path(@conn, "/flags/#{url_safe(@flag.name)}/groups/#{url_safe(@gate.for)}") %>" method="post" class="fwf-inline-toggle">
         <input type="hidden" name="_method" value="PATCH">
         <input type="hidden" name="_csrf_token" value="<%= @conn.assigns[:csrf_token] %>">
 
@@ -21,10 +21,10 @@
       </form>
     </div>
     <div class="col-lg-1 col-md-1 col-sm-2 col-3 text-right">
-      <form action="<%= path(@conn, "/flags/#{html_safe(@flag.name)}/groups/#{@gate.for}") %>" method="post" class="fwf-inline-toggle">
+      <form action="<%= path(@conn, "/flags/#{url_safe(@flag.name)}/groups/#{url_safe(@gate.for)}") %>" method="post" class="fwf-inline-toggle">
         <input type="hidden" name="_method" value="DELETE">
         <input type="hidden" name="_csrf_token" value="<%= @conn.assigns[:csrf_token] %>">
-        <button type="submit" class="btn btn-sm btn-secondary" data-confirm="Are you sure you want to clear group '<%= @gate.for %>'?">Clear</button>
+        <button type="submit" class="btn btn-sm btn-secondary" data-confirm="Are you sure you want to clear group '<%= html_safe(@gate.for) %>'?">Clear</button>
       </form>
     </div>
   </div>

diff --git a/lib/fun_with_flags/ui/templates/rows/_new_actor.html.eex b/lib/fun_with_flags/ui/templates/rows/_new_actor.html.eex
@@ -1,5 +1,5 @@
 <span>
-  <form id="fwf-new-actor-form" action="<%= path(@conn, "/flags/#{html_safe(@flag.name)}/actors") %>" method="post" class="form-inline">
+  <form id="fwf-new-actor-form" action="<%= path(@conn, "/flags/#{url_safe(@flag.name)}/actors") %>" method="post" class="form-inline">
     <input type="hidden" name="_csrf_token" value="<%= @conn.assigns[:csrf_token] %>">
     <%= if assigns[:error_message] do %>
       <div class="input-group mr-2 has-danger fwf-wide-input">

diff --git a/lib/fun_with_flags/ui/templates/rows/_new_group.html.eex b/lib/fun_with_flags/ui/templates/rows/_new_group.html.eex
@@ -1,5 +1,5 @@
 <span>
-  <form id="fwf-new-group-form" action="<%= path(@conn, "/flags/#{html_safe(@flag.name)}/groups") %>" method="post" class="form-inline">
+  <form id="fwf-new-group-form" action="<%= path(@conn, "/flags/#{url_safe(@flag.name)}/groups") %>" method="post" class="form-inline">
     <input type="hidden" name="_csrf_token" value="<%= @conn.assigns[:csrf_token] %>">
     <%= if assigns[:error_message] do %>
       <div class="input-group mr-2 has-danger fwf-wide-input">

diff --git a/lib/fun_with_flags/ui/templates/rows/_percentage.html.eex b/lib/fun_with_flags/ui/templates/rows/_percentage.html.eex
@@ -14,11 +14,11 @@
     </div>
 
     <div class="col-lg-5 col-md-5 col-sm-5 col-5 text-left">
-      raw value: <code><%= @gate.for %></code>
+      raw value: <code><%= html_safe(@gate.for) %></code>
     </div>
 
     <div class="col-lg-1 col-md-1 col-sm-2 col-3 text-right">
-      <form action="<%= path(@conn, "/flags/#{html_safe(@flag.name)}/percentage") %>" method="post" class="float-right">
+      <form action="<%= path(@conn, "/flags/#{url_safe(@flag.name)}/percentage") %>" method="post" class="float-right">
         <input type="hidden" name="_method" value="DELETE">
         <input type="hidden" name="_csrf_token" value="<%= @conn.assigns[:csrf_token] %>">
         <button type="submit" class="btn btn-sm btn-secondary" data-confirm="Are you sure you want to clear the '<%= human_name %>' gate?">Clear</button>

diff --git a/lib/fun_with_flags/ui/templates/rows/_percentage_form.html.eex b/lib/fun_with_flags/ui/templates/rows/_percentage_form.html.eex
@@ -2,7 +2,7 @@
   action = if is_nil(@gate), do: "Add", else: "Update"
 %>
 <span>
-  <form id="fwf-new-group-form" action="<%= path(@conn, "/flags/#{html_safe(@flag.name)}/percentage") %>" method="post" class="form-inline">
+  <form id="fwf-new-group-form" action="<%= path(@conn, "/flags/#{url_safe(@flag.name)}/percentage") %>" method="post" class="form-inline">
     <input type="hidden" name="_csrf_token" value="<%= @conn.assigns[:csrf_token] %>">
     <%= if assigns[:error_message] do %>
       <div class="input-group mr-2 has-danger fwf-wide-input">

diff --git a/test/fun_with_flags/ui/templates_test.exs b/test/fun_with_flags/ui/templates_test.exs
@@ -144,6 +144,20 @@ defmodule FunWithFlags.UI.TemplatesTest do
       assert String.contains?(out, ~s{<div id="group_rocks"})
       assert String.contains?(out, ~s{<form action="/pear/flags/avocado/groups/rocks" method="post"})
     end
+
+    test "with actors and groups it contains their rows with escaped HTML and URLs", %{conn: conn, flag: flag} do
+      group_gate = %Gate{type: :group, for: :rocks, enabled: true}
+      actor_gate = %Gate{type: :actor, for: "moss:<h1>123</h1>", enabled: true}
+      flag = %Flag{flag | gates: [actor_gate, group_gate]}
+
+      out = Templates.details(conn: conn, flag: flag)
+
+      assert String.contains?(out, ~s{<div id="actor_moss:&lt;h1&gt;123&lt;/h1&gt;"})
+      assert String.contains?(out, ~s{<form action="/pear/flags/avocado/actors/moss:%3Ch1%3E123%3C/h1%3E" method="post"})
+
+      assert String.contains?(out, ~s{<div id="group_rocks"})
+      assert String.contains?(out, ~s{<form action="/pear/flags/avocado/groups/rocks" method="post"})
+    end
   end