diff --git a/CHANGELOG.md b/CHANGELOG.md index c63ad87..81b51c9 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/lib/claper/application.ex b/lib/claper/application.ex index 80100d6..865e69e 100644 --- a/lib/claper/application.ex +++ b/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, diff --git a/lib/claper/embeds/embed.ex b/lib/claper/embeds/embed.ex index f05bc9d..bff2071 100644 --- a/lib/claper/embeds/embed.ex +++ b/lib/claper/embeds/embed.ex @@ -97,9 +97,62 @@ defmodule Claper.Embeds.Embed do |> validate_format(:content, ~r//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/]*?(?:\/>|>[\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, "\"", """)}") + 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: "", else: "" + end end diff --git a/lib/claper/rate_limit.ex b/lib/claper/rate_limit.ex new file mode 100644 index 0000000..95ba4a3 --- /dev/null +++ b/lib/claper/rate_limit.ex @@ -0,0 +1,3 @@ +defmodule Claper.RateLimit do + use Hammer, backend: :ets +end diff --git a/lib/claper/tasks/converter.ex b/lib/claper/tasks/converter.ex index ff8baf7..f70bc2c 100644 --- a/lib/claper/tasks/converter.ex +++ b/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 diff --git a/lib/claper_web/controllers/stat_controller.ex b/lib/claper_web/controllers/stat_controller.ex index 8b4f4bf..81e310c 100644 --- a/lib/claper_web/controllers/stat_controller.ex +++ b/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 """ diff --git a/lib/claper_web/helpers.ex b/lib/claper_web/helpers.ex index 99c5598..0b6ad42 100644 --- a/lib/claper_web/helpers.ex +++ b/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(#{url}) + ~s(#{escaped}) ) text -> diff --git a/lib/claper_web/live/admin_live/dashboard_live.ex b/lib/claper_web/live/admin_live/dashboard_live.ex index af6e07d..eb8526b 100644 --- a/lib/claper_web/live/admin_live/dashboard_live.ex +++ b/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 diff --git a/lib/claper_web/live/admin_live/table_actions_component.ex b/lib/claper_web/live/admin_live/table_actions_component.ex index 4ccd225..c8c0620 100644 --- a/lib/claper_web/live/admin_live/table_actions_component.ex +++ b/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)} diff --git a/lib/claper_web/live/event_live/form_component.ex b/lib/claper_web/live/event_live/form_component.ex index 2862ad5..e70ac45 100644 --- a/lib/claper_web/live/event_live/form_component.ex +++ b/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( diff --git a/lib/claper_web/live/event_live/manage.ex b/lib/claper_web/live/event_live/manage.ex index e5c3b80..ab5951f 100644 --- a/lib/claper_web/live/event_live/manage.ex +++ b/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 diff --git a/lib/claper_web/live/event_live/show.ex b/lib/claper_web/live/event_live/show.ex index 6b5a512..116f9cf 100644 --- a/lib/claper_web/live/event_live/show.ex +++ b/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) diff --git a/lib/claper_web/plugs/rate_limit_plug.ex b/lib/claper_web/plugs/rate_limit_plug.ex new file mode 100644 index 0000000..6a617e4 --- /dev/null +++ b/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 diff --git a/lib/claper_web/router.ex b/lib/claper_web/router.ex index bd049f3..67c93ae 100644 --- a/lib/claper_web/router.ex +++ b/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) diff --git a/mix.exs b/mix.exs index d8b75df..27a2ff5 100644 --- a/mix.exs +++ b/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 diff --git a/mix.lock b/mix.lock index 63b4b63..4595508 100644 --- a/mix.lock +++ b/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"},