Fix critical security vulnerabilities (#211)

* Fix critical security vulnerabilities

Address 5 critical findings from security audit:
- Sanitize custom embed HTML to prevent stored XSS (strip all non-iframe content)
- Escape URLs in format_body/1 to prevent reflected XSS via post messages
- Add authorization check to form export endpoint (IDOR fix)
- Replace String.to_atom/1 on user input with explicit whitelists (8 locations)
- Add IP-based rate limiting on authentication endpoints via Hammer

* Start rate limiter before endpoint in supervision tree

* Update CHANGELOG

CHANGELOG.md

@@ -1,5 +1,13 @@
 ### v.2.4.1
 
+### Security
+
+- Fix stored XSS vulnerability in custom embed iframes via input sanitization with attribute whitelisting
+- Fix XSS vulnerability in URL link formatting by escaping user-submitted URLs
+- Fix IDOR on form export endpoint by adding authorization check
+- Fix atom exhaustion DoS by replacing `String.to_atom/1` on user input with explicit whitelists (8 locations)
+- Add rate limiting on authentication endpoints using Hammer 7.0
+
 ### Fixes and improvements
 
 - Improve SMTP config and handling (#197)

lib/claper/application.ex

@@ -19,6 +19,8 @@ defmodule Claper.Application do
       ClaperWeb.Telemetry,
       # Start the PubSub system
       {Phoenix.PubSub, name: Claper.PubSub},
+      # Start the rate limiter before the endpoint accepts requests
+      Claper.RateLimit,
       # Start the Endpoint (http/https)
       ClaperWeb.Presence,
       ClaperWeb.Endpoint,

lib/claper/embeds/embed.ex

@@ -97,9 +97,62 @@ defmodule Claper.Embeds.Embed do
         |> validate_format(:content, ~r/<iframe.*?<\/iframe>/s,
           message: gettext("Please enter valid HTML content with an iframe tag")
         )
+        |> sanitize_custom_embed()
 
       _ ->
         changeset
     end
   end
+
+  @allowed_iframe_attrs ~w(src width height frameborder allow allowfullscreen style title loading referrerpolicy sandbox)
+
+  defp sanitize_custom_embed(%{valid?: false} = changeset), do: changeset
+
+  defp sanitize_custom_embed(changeset) do
+    content = get_field(changeset, :content)
+
+    case Regex.run(~r/<iframe\s[^>]*?(?:\/>|>[\s\S]*?<\/iframe>)/i, content) do
+      [iframe_tag] ->
+        put_change(changeset, :content, sanitize_iframe_tag(iframe_tag))
+
+      _ ->
+        add_error(
+          changeset,
+          :content,
+          gettext("Please enter valid HTML content with an iframe tag")
+        )
+    end
+  end
+
+  @allowed_boolean_attrs ~w(allowfullscreen sandbox)
+
+  defp sanitize_iframe_tag(iframe_tag) do
+    # Extract key="value" attributes
+    value_attrs =
+      Regex.scan(~r/([\w-]+)\s*=\s*(?:"([^"]*?)"|'([^']*?)')/i, iframe_tag)
+      |> Enum.filter(fn [_, name | _] -> String.downcase(name) in @allowed_iframe_attrs end)
+      |> Enum.reject(fn [_, name, value | _] ->
+        String.downcase(name) == "src" and String.trim(value) =~ ~r/^javascript:/i
+      end)
+      |> Enum.map(fn [_, name, value | _rest] ->
+        ~s(#{String.downcase(name)}="#{String.replace(value, "\"", "&quot;")}")
+      end)
+
+    # Extract standalone boolean attributes (e.g., allowfullscreen)
+    value_attr_names =
+      Regex.scan(~r/([\w-]+)\s*=/i, iframe_tag)
+      |> Enum.map(fn [_, name] -> String.downcase(name) end)
+      |> MapSet.new()
+
+    boolean_attrs =
+      Regex.scan(~r/\s([\w-]+)(?=[\s>\/])/i, iframe_tag)
+      |> Enum.map(fn [_, name] -> String.downcase(name) end)
+      |> Enum.filter(&(&1 in @allowed_boolean_attrs))
+      |> Enum.reject(&MapSet.member?(value_attr_names, &1))
+      |> Enum.uniq()
+
+    all_attrs = Enum.join(value_attrs ++ boolean_attrs, " ")
+
+    if all_attrs == "", do: "<iframe></iframe>", else: "<iframe #{all_attrs}></iframe>"
+  end
 end

lib/claper/rate_limit.ex

@@ -0,0 +1,3 @@
+defmodule Claper.RateLimit do
+  use Hammer, backend: :ets
+end

lib/claper/tasks/converter.ex

@@ -30,7 +30,14 @@ defmodule Claper.Tasks.Converter do
 
     IO.puts("Starting conversion for #{hash}... (copy: #{is_copy})")
 
-    file_to_pdf(String.to_atom(ext), path, file)
+    ext_atom =
+      case ext do
+        "ppt" -> :ppt
+        "pptx" -> :pptx
+        other -> other
+      end
+
+    file_to_pdf(ext_atom, path, file)
     |> pdf_to_jpg(path, presentation, user_id)
     |> jpg_upload(hash, path, presentation, user_id, is_copy)
   end

lib/claper_web/controllers/stat_controller.ex

@@ -10,20 +10,27 @@ defmodule ClaperWeb.StatController do
   @doc """
   Exports form submissions as a CSV file.
   """
-  def export_form(conn, %{"form_id" => form_id}) do
-    form = Forms.get_form!(form_id, [:form_submits])
-    headers = form.fields |> Enum.map(& &1.name)
+  def export_form(%{assigns: %{current_user: current_user}} = conn, %{"form_id" => form_id}) do
+    with form <- Forms.get_form!(form_id, [:form_submits]),
+         presentation_file <-
+           Presentations.get_presentation_file!(form.presentation_file_id, [:event]),
+         event <- presentation_file.event,
+         :ok <- authorize_event_access(current_user, event) do
+      headers = form.fields |> Enum.map(& &1.name)
 
-    data =
-      form.form_submits
-      |> Enum.map(fn submit ->
-        form.fields
-        |> Enum.map(fn field ->
-          Map.get(submit.response, field.name, "")
+      data =
+        form.form_submits
+        |> Enum.map(fn submit ->
+          form.fields
+          |> Enum.map(fn field ->
+            Map.get(submit.response, field.name, "")
+          end)
         end)
-      end)
 
-    export_as_csv(conn, headers, data, "form-#{sanitize(form.title)}")
+      export_as_csv(conn, headers, data, "form-#{sanitize(form.title)}")
+    else
+      :unauthorized -> send_resp(conn, 403, "Forbidden")
+    end
   end
 
   @doc """

lib/claper_web/helpers.ex

@@ -6,8 +6,10 @@ defmodule ClaperWeb.Helpers do
     |> String.split(url_regex, include_captures: true)
     |> Enum.map(fn
       "http" <> _rest = url ->
+        escaped = url |> Phoenix.HTML.html_escape() |> Phoenix.HTML.safe_to_string()
+
         Phoenix.HTML.raw(
-          ~s(<a href="#{url}" target="_blank" class="cursor-pointer text-primary-500 hover:underline font-medium">#{url}</a>)
+          ~s(<a href="#{escaped}" target="_blank" class="cursor-pointer text-primary-500 hover:underline font-medium">#{escaped}</a>)
         )
 
       text ->

lib/claper_web/live/admin_live/dashboard_live.ex

@@ -34,7 +34,13 @@ defmodule ClaperWeb.AdminLive.DashboardLive do
 
   @impl true
   def handle_event("change_period", %{"period" => period}, socket) do
-    period_atom = String.to_atom(period)
+    period_atom =
+      case period do
+        "day" -> :day
+        "week" -> :week
+        "month" -> :month
+        _ -> :day
+      end
 
     days_back =
       case period_atom do

lib/claper_web/live/admin_live/table_actions_component.ex

@@ -231,10 +231,16 @@ defmodule ClaperWeb.AdminLive.TableActionsComponent do
     action = Enum.find(socket.assigns.dropdown_actions, &(&1.key == action_key))
 
     if action do
-      send(
-        self(),
-        {:table_action, String.to_atom(action_key), socket.assigns.item, socket.assigns.item_id}
-      )
+      try do
+        action_atom = String.to_existing_atom(action_key)
+
+        send(
+          self(),
+          {:table_action, action_atom, socket.assigns.item, socket.assigns.item_id}
+        )
+      rescue
+        ArgumentError -> :ok
+      end
     end
 
     {:noreply, assign(socket, dropdown_open: false)}

lib/claper_web/live/event_live/form_component.ex

@@ -61,7 +61,7 @@ defmodule ClaperWeb.EventLive.FormComponent do
                       form={f}
                       labelClass="text-white"
                       fieldClass="bg-gray-700 text-white"
-                      key={String.to_atom(field.name)}
+                      key={safe_field_atom(field.name)}
                       name={field.name}
                       required={field.required}
                       value={
@@ -75,7 +75,7 @@ defmodule ClaperWeb.EventLive.FormComponent do
                       form={f}
                       labelClass="text-white"
                       fieldClass="bg-gray-700 text-white"
-                      key={String.to_atom(field.name)}
+                      key={safe_field_atom(field.name)}
                       name={field.name}
                       required={field.required}
                       value={
@@ -170,6 +170,15 @@ defmodule ClaperWeb.EventLive.FormComponent do
     end
   end
 
+  defp safe_field_atom(name) when is_binary(name) do
+    String.to_existing_atom(name)
+  rescue
+    ArgumentError ->
+      if Regex.match?(~r/^[\w]{1,100}$/, name),
+        do: String.to_atom(name),
+        else: :invalid_field
+  end
+
   def toggle_form(js \\ %JS{}) do
     js
     |> JS.toggle(

lib/claper_web/live/event_live/manage.ex

@@ -711,36 +711,42 @@ defmodule ClaperWeb.EventLive.Manage do
 
   @impl true
   def handle_event("list-tab", %{"tab" => tab}, socket) do
-    socket = assign(socket, :list_tab, String.to_atom(tab))
-
-    socket =
+    {tab_atom, socket} =
       case tab do
         "posts" ->
-          socket
-          |> stream(:posts, list_all_posts(socket, socket.assigns.event.uuid), reset: true)
+          {:posts,
+           stream(socket, :posts, list_all_posts(socket, socket.assigns.event.uuid), reset: true)}
 
         "questions" ->
-          socket
-          |> stream(:questions, list_all_questions(socket, socket.assigns.event.uuid),
-            reset: true
-          )
+          {:questions,
+           stream(socket, :questions, list_all_questions(socket, socket.assigns.event.uuid),
+             reset: true
+           )}
 
         "forms" ->
-          stream(
-            socket,
-            :form_submits,
-            list_form_submits(socket, socket.assigns.event.presentation_file.id),
-            reset: true
-          )
+          {:forms,
+           stream(
+             socket,
+             :form_submits,
+             list_form_submits(socket, socket.assigns.event.presentation_file.id),
+             reset: true
+           )}
 
         "pinned_posts" ->
-          socket
-          |> stream(:pinned_posts, list_pinned_posts(socket, socket.assigns.event.uuid),
-            reset: true
-          )
+          {:pinned_posts,
+           stream(
+             socket,
+             :pinned_posts,
+             list_pinned_posts(socket, socket.assigns.event.uuid),
+             reset: true
+           )}
+
+        _ ->
+          {:posts,
+           stream(socket, :posts, list_all_posts(socket, socket.assigns.event.uuid), reset: true)}
       end
 
-    {:noreply, socket}
+    {:noreply, assign(socket, :list_tab, tab_atom)}
   end
 
   @impl true
@@ -945,7 +951,13 @@ defmodule ClaperWeb.EventLive.Manage do
   end
 
   defp list_all_questions(_socket, event_id, sort \\ "date") do
-    Claper.Posts.list_questions(event_id, [:event, :reactions], String.to_atom(sort))
+    sort_atom =
+      case sort do
+        "likes" -> :likes
+        _ -> :date
+      end
+
+    Claper.Posts.list_questions(event_id, [:event, :reactions], sort_atom)
     |> Enum.filter(&(ClaperWeb.Helpers.body_without_links(&1.body) =~ "?"))
   end
 

lib/claper_web/live/event_live/show.ex

@@ -7,6 +7,19 @@ defmodule ClaperWeb.EventLive.Show do
 
   on_mount(ClaperWeb.AttendeeLiveAuth)
 
+  @global_react_types %{
+    "heart" => :heart,
+    "clap" => :clap,
+    "hundred" => :hundred,
+    "raisehand" => :raisehand
+  }
+
+  @reaction_fields %{
+    like: {:like_count, :like_posts},
+    love: {:love_count, :love_posts},
+    lol: {:lol_count, :lol_posts}
+  }
+
   @impl true
   def mount(%{"code" => code}, session, socket) do
     with %{"locale" => locale} <- session do
@@ -422,13 +435,19 @@ defmodule ClaperWeb.EventLive.Show do
         %{"type" => type},
         socket
       ) do
-    Phoenix.PubSub.broadcast(
-      Claper.PubSub,
-      "event:#{socket.assigns.event.uuid}",
-      {:react, String.to_atom(type)}
-    )
+    case Map.get(@global_react_types, type) do
+      nil ->
+        {:noreply, socket}
 
-    {:noreply, socket}
+      type_atom ->
+        Phoenix.PubSub.broadcast(
+          Claper.PubSub,
+          "event:#{socket.assigns.event.uuid}",
+          {:react, type_atom}
+        )
+
+        {:noreply, socket}
+    end
   end
 
   @impl true
@@ -752,8 +771,7 @@ defmodule ClaperWeb.EventLive.Show do
   defp add_reaction(socket, post_id, params, type) do
     with post <- Posts.get_post!(post_id, [:event]),
          {:ok, _} <- Posts.create_reaction(Map.merge(params, %{post: post})) do
-      count_field = String.to_atom("#{type}_count")
-      posts_field = String.to_atom("#{type}_posts")
+      {count_field, posts_field} = @reaction_fields[type]
 
       {:ok, _} = Posts.update_post(post, %{count_field => Map.get(post, count_field) + 1})
       update(socket, posts_field, fn posts -> [post.id | posts] end)
@@ -763,8 +781,7 @@ defmodule ClaperWeb.EventLive.Show do
   defp remove_reaction(socket, post_id, params, type) do
     with post <- Posts.get_post!(post_id, [:event]),
          {:ok, _} <- Posts.delete_reaction(Map.merge(params, %{post: post})) do
-      count_field = String.to_atom("#{type}_count")
-      posts_field = String.to_atom("#{type}_posts")
+      {count_field, posts_field} = @reaction_fields[type]
 
       {:ok, _} = Posts.update_post(post, %{count_field => Map.get(post, count_field) - 1})
       update(socket, posts_field, fn posts -> List.delete(posts, post.id) end)

lib/claper_web/plugs/rate_limit_plug.ex

@@ -0,0 +1,34 @@
+defmodule ClaperWeb.Plugs.RateLimitPlug do
+  @moduledoc """
+  Plug for rate limiting requests based on client IP address.
+
+  ## Usage
+
+      plug ClaperWeb.Plugs.RateLimitPlug, max_requests: 10, interval_ms: 60_000, prefix: "login"
+  """
+  import Plug.Conn
+
+  def init(opts) do
+    %{
+      max_requests: Keyword.get(opts, :max_requests, 10),
+      interval_ms: Keyword.get(opts, :interval_ms, 60_000),
+      prefix: Keyword.get(opts, :prefix, "default")
+    }
+  end
+
+  def call(conn, %{max_requests: max_requests, interval_ms: interval_ms, prefix: prefix}) do
+    ip = conn.remote_ip |> :inet.ntoa() |> to_string()
+    key = "#{prefix}:#{ip}"
+
+    case Claper.RateLimit.hit(key, interval_ms, max_requests) do
+      {:allow, _count} ->
+        conn
+
+      {:deny, _retry_after} ->
+        conn
+        |> put_resp_content_type("text/plain")
+        |> send_resp(429, "Too Many Requests")
+        |> halt()
+    end
+  end
+end

lib/claper_web/router.ex

@@ -35,6 +35,13 @@ defmodule ClaperWeb.Router do
     plug(:accepts, ["json"])
   end
 
+  pipeline :rate_limit_auth do
+    plug ClaperWeb.Plugs.RateLimitPlug,
+      max_requests: 10,
+      interval_ms: 60_000,
+      prefix: "auth"
+  end
+
   # Manage attendee_identifier in requests
   pipeline :attendee_registration do
     plug(:attendee_identifier)
@@ -120,7 +127,7 @@ defmodule ClaperWeb.Router do
 
   ## Authentication routes
   scope "/", ClaperWeb do
-    pipe_through([:browser, :redirect_if_user_is_authenticated])
+    pipe_through([:browser, :redirect_if_user_is_authenticated, :rate_limit_auth])
 
     get("/users/register", UserRegistrationController, :new)
     post("/users/register", UserRegistrationController, :create)

mix.exs

@@ -114,6 +114,7 @@ defmodule Claper.MixProject do
       {:uuid, "~> 1.1"},
       {:oidcc, "~> 3.5"},
       {:oban, "~> 2.19"},
+      {:hammer, "~> 7.0"},
       {:tailwind, "~> 0.3", runtime: Mix.env() == :dev}
     ]
   end

mix.lock

@@ -25,6 +25,7 @@
   "floki": {:hex, :floki, "0.38.0", "62b642386fa3f2f90713f6e231da0fa3256e41ef1089f83b6ceac7a3fd3abf33", [:mix], [], "hexpm", "a5943ee91e93fb2d635b612caf5508e36d37548e84928463ef9dd986f0d1abd9"},
   "gen_smtp": {:hex, :gen_smtp, "1.3.0", "62c3d91f0dcf6ce9db71bcb6881d7ad0d1d834c7f38c13fa8e952f4104a8442e", [:rebar3], [{:ranch, ">= 1.8.0", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "0b73fbf069864ecbce02fe653b16d3f35fd889d0fdd4e14527675565c39d84e6"},
   "gettext": {:hex, :gettext, "0.26.2", "5978aa7b21fada6deabf1f6341ddba50bc69c999e812211903b169799208f2a8", [:mix], [{:expo, "~> 0.5.1 or ~> 1.0", [hex: :expo, repo: "hexpm", optional: false]}], "hexpm", "aa978504bcf76511efdc22d580ba08e2279caab1066b76bb9aa81c4a1e0a32a5"},
+  "hammer": {:hex, :hammer, "7.2.0", "73113eca87f0fd20a6d3679c1182e8c4c1778266f61de4e9dc8c589dee156c30", [:mix], [], "hexpm", "c50fa865ddfe7b3d4f8a6941f56940679e02a9a1465b00668a95d140b101d828"},
   "hashids": {:hex, :hashids, "2.1.0", "aabbcc4f9fa0b460cc6ef629f5bcbc35e7e87b382fee79f9c50be40b86574288", [:mix], [], "hexpm", "172163b1642d415881ef1c6e1f1be12d4e92b0711d5bbbd8854f82a1ce32d60b"},
   "hpax": {:hex, :hpax, "1.0.3", "ed67ef51ad4df91e75cc6a1494f851850c0bd98ebc0be6e81b026e765ee535aa", [:mix], [], "hexpm", "8eab6e1cfa8d5918c2ce4ba43588e894af35dbd8e91e6e55c817bca5847df34a"},
   "jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"},
@@ -55,6 +56,7 @@
   "plug": {:hex, :plug, "1.18.1", "5067f26f7745b7e31bc3368bc1a2b818b9779faa959b49c934c17730efc911cf", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "57a57db70df2b422b564437d2d33cf8d33cd16339c1edb190cd11b1a3a546cc2"},
   "plug_cowboy": {:hex, :plug_cowboy, "2.7.4", "729c752d17cf364e2b8da5bdb34fb5804f56251e88bb602aff48ae0bd8673d11", [:mix], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:cowboy_telemetry, "~> 0.3", [hex: :cowboy_telemetry, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "9b85632bd7012615bae0a5d70084deb1b25d2bcbb32cab82d1e9a1e023168aa3"},
   "plug_crypto": {:hex, :plug_crypto, "2.1.1", "19bda8184399cb24afa10be734f84a16ea0a2bc65054e23a62bb10f06bc89491", [:mix], [], "hexpm", "6470bce6ffe41c8bd497612ffde1a7e4af67f36a15eea5f921af71cf3e11247c"},
+  "poolboy": {:hex, :poolboy, "1.5.2", "392b007a1693a64540cead79830443abf5762f5d30cf50bc95cb2c1aaafa006b", [:rebar3], [], "hexpm", "dad79704ce5440f3d5a3681c8590b9dc25d1a561e8f5a9c995281012860901e3"},
   "porcelain": {:hex, :porcelain, "2.0.3", "2d77b17d1f21fed875b8c5ecba72a01533db2013bd2e5e62c6d286c029150fdc", [:mix], [], "hexpm", "dc996ab8fadbc09912c787c7ab8673065e50ea1a6245177b0c24569013d23620"},
   "postgrex": {:hex, :postgrex, "0.20.0", "363ed03ab4757f6bc47942eff7720640795eb557e1935951c1626f0d303a3aed", [:mix], [{:db_connection, "~> 2.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "d36ef8b36f323d29505314f704e21a1a038e2dc387c6409ee0cd24144e187c0f"},
   "ranch": {:hex, :ranch, "2.2.0", "25528f82bc8d7c6152c57666ca99ec716510fe0925cb188172f41ce93117b1b0", [:make, :rebar3], [], "hexpm", "fa0b99a1780c80218a4197a59ea8d3bdae32fbff7e88527d7d8a4787eff4f8e7"},