Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
2 changes: 2 additions & 0 deletions lib/claper/application.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
53 changes: 53 additions & 0 deletions lib/claper/embeds/embed.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 3 additions & 0 deletions lib/claper/rate_limit.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
defmodule Claper.RateLimit do
use Hammer, backend: :ets
end
9 changes: 8 additions & 1 deletion lib/claper/tasks/converter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 19 additions & 12 deletions lib/claper_web/controllers/stat_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)

data =
form.form_submits
|> Enum.map(fn submit ->
form.fields
|> Enum.map(fn field ->
Map.get(submit.response, field.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, "")
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 """
Expand Down
4 changes: 3 additions & 1 deletion lib/claper_web/helpers.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand Down
8 changes: 7 additions & 1 deletion lib/claper_web/live/admin_live/dashboard_live.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 10 additions & 4 deletions lib/claper_web/live/admin_live/table_actions_component.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand Down
13 changes: 11 additions & 2 deletions lib/claper_web/live/event_live/form_component.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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={
Expand All @@ -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={
Expand Down Expand Up @@ -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(
Expand Down
54 changes: 33 additions & 21 deletions lib/claper_web/live/event_live/manage.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
37 changes: 27 additions & 10 deletions lib/claper_web/live/event_live/show.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
Loading
Loading