diff --git a/copi.owasp.org/lib/copi/cornucopia/game.ex b/copi.owasp.org/lib/copi/cornucopia/game.ex index 652eccec2..52ed3d126 100644 --- a/copi.owasp.org/lib/copi/cornucopia/game.ex +++ b/copi.owasp.org/lib/copi/cornucopia/game.ex @@ -1,4 +1,4 @@ -defmodule Copi.Cornucopia.Game do + defmodule Copi.Cornucopia.Game do use Ecto.Schema import Ecto.Changeset @@ -32,6 +32,9 @@ defmodule Copi.Cornucopia.Game do game |> cast(attrs, [:name, :created_at, :edition, :started_at, :finished_at, :rounds_played, :suits, :round_open]) |> validate_required([:name], message: "No really, give your game session a name") + |> validate_length(:name, min: 1, max: 100) + |> sanitize_name(:name) + |> validate_name(:name) end def continue_vote_count(game) do @@ -62,4 +65,62 @@ defmodule Copi.Cornucopia.Game do Enum.count(players_still_to_play) > 0 end + + # Sanitization functions to remove dangerous content + defp sanitize_name(changeset, field) do + case Ecto.Changeset.get_field(changeset, field) do + nil -> changeset + name when is_binary(name) -> + sanitized = sanitize_name_string(name) + Ecto.Changeset.put_change(changeset, field, sanitized) + _ -> changeset + end + end + + defp sanitize_name_string(name) do + name + |> remove_html_tags() + |> remove_javascript() + |> remove_data_attributes() + end + + defp remove_html_tags(name) do + Regex.replace(~r/<[^>]*>/, name, "") + end + + defp remove_javascript(name) do + Regex.replace(~r/]*>.*?<\/script>/si, name, "") + end + + defp remove_data_attributes(name) do + Regex.replace(~r/\s+data-[^=]*="[^"]*"/, name, "") + end + + # Input validation and sanitization to prevent XSS + defp validate_name(changeset, field) do + case Ecto.Changeset.get_field(changeset, field) do + nil -> changeset + name when is_binary(name) -> + if valid_name?(name) do + changeset + else + Ecto.Changeset.add_error( + changeset, + field, + "contains unsafe content. Please use only letters, numbers, spaces, and basic punctuation." + ) + end + _ -> + changeset + end + end + + defp valid_name?(name) when is_binary(name) and name != "" do + String.match?( + name, + ~r/^[\x{0600}-\x{06FF}\x{0750}-\x{077F}\x{08A0}-\x{08FF}\x{FB50}-\x{FDFF}\x{FE70}-\x{FEFF}\x{FDF2}\x{FDF3}\x{FDF4}\x{FDFD}\x{3040}-\x{309F}\x{30A0}-\x{30FF}\x{4E00}-\x{9FFF}\x{FF66}-\x{FF9F}ー々〆〤\x{3400}-\x{4DBF}\x{F900}-\x{FAFF}\x{0900}-\x{097F}\x{0621}-\x{064A}\x{0660}-\x{0669}\x{0E00}-\x{0E7F}«»฿ฯ๏๚๛\x{0400}-\x{04FF}\x{0500}-\x{052F}\x{2DE0}-\x{2DFF}\x{A640}-\x{A69F}Ю́ю́Я́я́\x{0370}-\x{03FF}\x{1F00}-\x{1FFF}A-Za-zÀ-ÖØ-öø-ÿĀ-ž0-9._\-'"ءآأؤإئابةتثجحخدذرزسشصضطظعغفقكلمنهوي ﷲﷴﷺﷻ ٠١٢٣٤٥٦٧٨٩ \s]+$/ux + ) + end + + defp valid_name?(_), do: false end diff --git a/copi.owasp.org/lib/copi/cornucopia/player.ex b/copi.owasp.org/lib/copi/cornucopia/player.ex index 1eeb7d4bf..1a6a96140 100644 --- a/copi.owasp.org/lib/copi/cornucopia/player.ex +++ b/copi.owasp.org/lib/copi/cornucopia/player.ex @@ -27,5 +27,65 @@ defmodule Copi.Cornucopia.Player do |> cast(attrs, [:name, :game_id]) |> validate_required([:name]) |> validate_length(:name, min: 1, max: 50) + |> sanitize_name(:name) + |> validate_name(:name) end + + # Sanitization functions to remove dangerous content + defp sanitize_name(changeset, field) do + case Ecto.Changeset.get_field(changeset, field) do + nil -> changeset + name when is_binary(name) -> + sanitized = sanitize_name_string(name) + Ecto.Changeset.put_change(changeset, field, sanitized) + _ -> changeset + end + end + + defp sanitize_name_string(name) do + name + |> remove_html_tags() + |> remove_javascript() + |> remove_data_attributes() + end + + defp remove_html_tags(name) do + Regex.replace(~r/<[^>]*>/, name, "") + end + + defp remove_javascript(name) do + Regex.replace(~r/]*>.*?<\/script>/si, name, "") + end + + defp remove_data_attributes(name) do + Regex.replace(~r/\s+data-[^=]*="[^"]*"/, name, "") + end + + # Input validation and sanitization to prevent XSS + defp validate_name(changeset, field) do + case Ecto.Changeset.get_field(changeset, field) do + nil -> changeset + name when is_binary(name) -> + if valid_name?(name) do + changeset + else + Ecto.Changeset.add_error( + changeset, + field, + "contains unsafe content. Please use only letters, numbers, spaces, and basic punctuation." + ) + end + _ -> + changeset + end + end + + defp valid_name?(name) when is_binary(name) and name != "" do + String.match?( + name, + ~r/^[\x{0600}-\x{06FF}\x{0750}-\x{077F}\x{08A0}-\x{08FF}\x{FB50}-\x{FDFF}\x{FE70}-\x{FEFF}\x{FDF2}\x{FDF3}\x{FDF4}\x{FDFD}\x{3040}-\x{309F}\x{30A0}-\x{30FF}\x{4E00}-\x{9FFF}\x{FF66}-\x{FF9F}ー々〆〤\x{3400}-\x{4DBF}\x{F900}-\x{FAFF}\x{0900}-\x{097F}\x{0621}-\x{064A}\x{0660}-\x{0669}\x{0E00}-\x{0E7F}«»฿ฯ๏๚๛\x{0400}-\x{04FF}\x{0500}-\x{052F}\x{2DE0}-\x{2DFF}\x{A640}-\x{A69F}Ю́ю́Я́я́\x{0370}-\x{03FF}\x{1F00}-\x{1FFF}A-Za-zÀ-ÖØ-öø-ÿĀ-ž0-9._\-'"ءآأؤإئابةتثجحخدذرزسشصضطظعغفقكلمنهوي ﷲﷴﷺﷻ ٠١٢٣٤٥٦٧٨٩ \s]+$/ux + ) + end + + defp valid_name?(_), do: false end diff --git a/copi.owasp.org/test/copi/cornucopia_test.exs b/copi.owasp.org/test/copi/cornucopia_test.exs index 3642f2a16..91cc7caa0 100644 --- a/copi.owasp.org/test/copi/cornucopia_test.exs +++ b/copi.owasp.org/test/copi/cornucopia_test.exs @@ -41,6 +41,44 @@ defmodule Copi.CornucopiaTest do assert {:error, %Ecto.Changeset{}} = Cornucopia.create_game(@invalid_attrs) end + test "create_game/1 rejects XSS payloads in name" do + # Test various XSS payloads + xss_payloads = [ + "", + "javascript:alert('xss')", + "", + "onload=alert('xss')", + "", + "data:text/html," + ] + + for payload <- xss_payloads do + attrs = %{name: payload, edition: "webapp"} + assert {:error, %Ecto.Changeset{}} = Cornucopia.create_game(attrs) + end + end + + test "create_game/1 accepts safe names" do + safe_names = [ + "My Game Session", + "Security Review 2024", + "Test Game-123", + "O'Connor's Security Game", + "Game \"Secure\" Session", + "لعبة الأمن", + "安全游戏", + "Игра Безопасности", + "Juego de Seguridad", + "Sécurité de Jeu", + "セキュリティゲーム" + ] + + for name <- safe_names do + attrs = %{name: name, edition: "webapp"} + assert {:ok, %Game{name: ^name}} = Cornucopia.create_game(attrs) + end + end + test "update_game/2 with valid data updates the game" do game = game_fixture() assert {:ok, %Game{} = game} = Cornucopia.update_game(game, @update_attrs) @@ -144,6 +182,47 @@ defmodule Copi.CornucopiaTest do assert {:error, %Ecto.Changeset{}} = Cornucopia.create_player(@invalid_attrs) end + test "create_player/1 rejects XSS payloads in name" do + # Test various XSS payloads + xss_payloads = [ + "", + "javascript:alert('xss')", + "", + "onload=alert('xss')", + "", + "data:text/html," + ] + + for payload <- xss_payloads do + attrs = %{name: payload, game_id: "00000000000000000000000001"} + assert {:error, %Ecto.Changeset{}} = Cornucopia.create_player(attrs) + end + end + + test "create_player/1 accepts safe names" do + {:ok, game} = Cornucopia.create_game(%{name: "Test Game", edition: "webapp"}) + + safe_names = [ + "John Doe", + "O'Connor", + "Jane Smith-Anderson", + "Player 123", + "العربية", + "中文", + "Русский", + "Español", + "Français", + "日本語", + "العربية-English", + "测试游戏" + ] + + for name <- safe_names do + attrs = %{name: name, game_id: game.id} + assert {:ok, %Player{name: ^name}} = Cornucopia.create_player(attrs) + end + end + test "update_player/2 with valid data updates the player" do player = player_fixture() assert {:ok, %Player{} = player} = Cornucopia.update_player(player, @update_attrs) diff --git a/copi.owasp.org/test/copi_web/live/player_live_test.exs b/copi.owasp.org/test/copi_web/live/player_live_test.exs index ff6ea9e5b..84723524d 100644 --- a/copi.owasp.org/test/copi_web/live/player_live_test.exs +++ b/copi.owasp.org/test/copi_web/live/player_live_test.exs @@ -135,13 +135,13 @@ defmodule CopiWeb.PlayerLiveTest do test "allows continue voting and resets votes on next round", %{conn: conn, player: player} do # Setup another player game_id = player.game_id - {:ok, other_player} = Cornucopia.create_player(%{name: "Other", game_id: game_id}) + {:ok, _other_player} = Cornucopia.create_player(%{name: "Other", game_id: game_id}) # Start game {:ok, game} = Cornucopia.Game.find(game_id) Copi.Repo.update!(Ecto.Changeset.change(game, started_at: DateTime.truncate(DateTime.utc_now(), :second))) - {:ok, show_live, html} = live(conn, "/games/#{game_id}/players/#{player.id}") + {:ok, show_live, _html} = live(conn, "/games/#{game_id}/players/#{player.id}") # Test continue voting show_live |> element("[phx-click=\"toggle_continue_vote\"]") |> render_click() diff --git a/copi.owasp.org/test/copi_web/plugs/rate_limiter_plug_test.exs b/copi.owasp.org/test/copi_web/plugs/rate_limiter_plug_test.exs index 74e361a45..b97951ed4 100644 --- a/copi.owasp.org/test/copi_web/plugs/rate_limiter_plug_test.exs +++ b/copi.owasp.org/test/copi_web/plugs/rate_limiter_plug_test.exs @@ -1,6 +1,7 @@ defmodule CopiWeb.Plugs.RateLimiterPlugTest do use ExUnit.Case, async: false - use Plug.Test + import Plug.Test + import Plug.Conn alias CopiWeb.Plugs.RateLimiterPlug alias Copi.RateLimiter