OWASP · sydseter · Apr 14, 2026 · Mar 5, 2026 · Mar 5, 2026 · Mar 6, 2026
@@ -158,9 +158,35 @@ defmodule Copi.Cornucopia do
 
   """
   def create_player(attrs \\ %{}) do
-    %Player{}
-    |> Player.changeset(attrs)
-    |> Repo.insert()
+    # V15.4: Use transaction with row locking to prevent TOCTOU race condition
+    # when checking if game has started before creating player
+    game_id = attrs["game_id"] || attrs[:game_id]
+
+    Repo.transaction(fn ->
+      # Lock the game row for update to prevent race conditions
+      case game_id && Repo.get(Game, game_id, lock: "FOR UPDATE") do
+        nil ->
+          # No game_id provided or game not found - let changeset validation handle it
+          case %Player{} |> Player.changeset(attrs) |> Repo.insert() do
+            {:ok, player} -> player
+            {:error, changeset} -> Repo.rollback(changeset)
+          end
+
+        %Game{started_at: started_at} when started_at != nil ->
+          Repo.rollback(:game_already_started)
+
+        %Game{} ->
+          case %Player{} |> Player.changeset(attrs) |> Repo.insert() do
+            {:ok, player} -> player
+            {:error, changeset} -> Repo.rollback(changeset)
+          end
+      end
+    end)
+    |> case do
+      {:ok, player} -> {:ok, player}
+      {:error, :game_already_started} -> {:error, :game_already_started}
+      {:error, %Ecto.Changeset{} = changeset} -> {:error, changeset}
+    end
   end
 
   @doc """

@@ -40,7 +40,6 @@ defmodule CopiWeb.GameLive.Show do
         {:error, _reason} ->
           {:noreply, redirect(socket, to: "/error")}
       end
-
     else
       {:error, _reason} ->
         {:noreply, redirect(socket, to: "/error")}

@@ -95,28 +95,53 @@ defmodule CopiWeb.PlayerLive.FormComponent do
 
   defp save_player(socket, :new, player_params) do
     ip = socket.assigns[:client_ip] || {127, 0, 0, 1}
+    game_id = player_params["game_id"]
 
-    case RateLimiter.check_rate(ip, :player_creation) do
-      {:ok, _remaining} ->
-        case Cornucopia.create_player(player_params) do
-          {:ok, player} ->
-            {:ok, updated_game} = Cornucopia.Game.find(socket.assigns.player.game_id)
-            CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game)
-
+    # V2.2: Re-fetch authoritative game state from DB at the trusted service layer
+    # to prevent bypassing the UI check via direct form submission or race conditions
+    case Cornucopia.Game.find(game_id) do
+      {:ok, game} when game.started_at != nil ->
+        {:noreply,
+         socket
+         |> put_flash(:error, "This game has already started. New players cannot join a game in progress.")
+         |> push_navigate(to: ~p"/games/#{game_id}")}
+
+      {:ok, _game} ->
+        case RateLimiter.check_rate(ip, :player_creation) do
+          {:ok, _remaining} ->
+            case Cornucopia.create_player(player_params) do
+              {:ok, player} ->
+                {:ok, updated_game} = Cornucopia.Game.find(socket.assigns.player.game_id)
+                CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game)
+
+                {:noreply,
+                 socket
+                 |> assign(:game, updated_game)
+                 |> push_navigate(to: ~p"/games/#{player.game_id}/players/#{player.id}")}
+
+              {:error, :game_already_started} ->
+                # V15.4: Race condition caught by transaction - game started between check and insert
+                {:noreply,
+                 socket
+                 |> put_flash(:error, "This game has already started. New players cannot join a game in progress.")
+                 |> push_navigate(to: ~p"/games")}
+
+              {:error, %Ecto.Changeset{} = changeset} ->
+                {:noreply, assign_form(socket, changeset)}
+            end
+
+          {:error, :rate_limit_exceeded} ->
             {:noreply,
              socket
-             |> assign(:game, updated_game)
-             |> push_navigate(to: ~p"/games/#{player.game_id}/players/#{player.id}")}
-
-          {:error, %Ecto.Changeset{} = changeset} ->
-            {:noreply, assign_form(socket, changeset)}
+             |> put_flash(:error, "Too many player creation attempts. Please try again later.")
+             |> assign_form(Cornucopia.change_player(socket.assigns.player))}
         end
 
-      {:error, :rate_limit_exceeded} ->
+      {:error, _} ->
         {:noreply,
          socket
-         |> put_flash(:error, "Too many player creation attempts. Please try again later.")
-         |> assign_form(Cornucopia.change_player(socket.assigns.player))}
+         |> put_flash(:error, "Game not found")
+         |> push_navigate(to: ~p"/games")}
     end
   end
 

@@ -7,12 +7,41 @@ defmodule CopiWeb.PlayerLive.Index do
   @impl true
   def mount(%{"game_id" => game_id}, session, socket) do
     ip = socket.assigns[:client_ip] || Map.get(session, "client_ip") || Copi.IPHelper.get_ip_from_socket(socket)
-    {:ok, assign(assign(socket, :client_ip, ip), players: list_players(game_id), game: Cornucopia.get_game!(game_id))}
+    game = Cornucopia.get_game!(game_id)
+
+    # V2.2: Block at mount — returning redirect from mount sends a true HTTP 302
+    # during the dead (static) render, before any HTML or WebSocket reaches the client.
+    if game.started_at do
+      {:ok,
+       socket
+       |> put_flash(:error, "This game has already started. New players cannot join a game in progress.")
+       |> redirect(to: ~p"/games")}
+    else
+      {:ok, assign(assign(socket, :client_ip, ip), players: list_players(game_id), game: game)}
+    end
   end
 
   @impl true
-  def handle_params(params, _url, socket) do
-    {:noreply, apply_action(socket, socket.assigns.live_action, params)}
+  def handle_params(%{"game_id" => game_id} = params, _url, socket) do
+    # Re-fetch game state for LiveView client-side navigations (when mount isn't called)
+    game = Cornucopia.get_game!(game_id)
+
+    # V2.2: Also check in handle_params for LiveView navigation scenarios
+    if game.started_at do
+      {:noreply,
+       socket
+       |> put_flash(:error, "This game has already started. New players cannot join a game in progress.")
+       |> redirect(to: ~p"/games")}
+    else
+      # Assign freshly loaded game and players for LiveView client-side navigations
+      players = list_players(game_id)
+
+      {:noreply,
+       socket
+       |> assign(:game, game)
+       |> assign(:players, players)
+       |> apply_action(socket.assigns.live_action, params)}
+    end
   end
 
   defp apply_action(socket, :edit, %{"id" => id}) do

@@ -88,8 +88,11 @@ defmodule Copi.CornucopiaTest do
     test "Game.majority_continue_votes_reached?/1 returns true when votes exceed half" do
       alias Copi.Cornucopia.Game
       alias Copi.Repo
-      game = game_fixture()
+      # Create game WITHOUT started_at so we can add players
+      {:ok, game} = Cornucopia.create_game(%{name: "majority vote test", edition: "webapp"})
       {:ok, created_player} = Cornucopia.create_player(%{name: "p1", game_id: game.id})
+      # Now start the game after player is created
+      {:ok, _} = Cornucopia.update_game(game, %{started_at: DateTime.truncate(DateTime.utc_now(), :second)})
       {:ok, reloaded} = Game.find(game.id)
       # 0 votes, 1 player → 0 > div(1,2)=0 → false
       refute Game.majority_continue_votes_reached?(reloaded)
@@ -108,7 +111,7 @@ defmodule Copi.CornucopiaTest do
     @valid_attrs %{name: "some name"}
     @update_attrs %{name: "some updated name"}
     @invalid_attrs %{name: nil}
-    @game_attrs %{created_at: "2010-04-17T14:00:00Z", edition: "webapp", finished_at: "2010-04-17T14:00:00Z", name: "some name", started_at: "2010-04-17T14:00:00Z"}
+    @game_attrs %{created_at: "2010-04-17T14:00:00Z", edition: "webapp", finished_at: "2010-04-17T14:00:00Z", name: "some name"}
 
     def player_fixture(attrs \\ %{}) do
       {:ok, player} =
@@ -166,6 +169,22 @@ defmodule Copi.CornucopiaTest do
       player = player_fixture()
       assert %Ecto.Changeset{} = Cornucopia.change_player(player)
     end
+
+    test "create_player/1 returns error when game has already started" do
+      {:ok, game} = Cornucopia.create_game(%{name: "started game", edition: "webapp"})
+      # Start the game by setting started_at
+      {:ok, started_game} = Cornucopia.update_game(game, %{started_at: DateTime.truncate(DateTime.utc_now(), :second)})
+
+      assert {:error, :game_already_started} = Cornucopia.create_player(%{name: "Late Player", game_id: started_game.id})
+    end
+
+    test "create_player/1 succeeds when game has not started" do
+      {:ok, game} = Cornucopia.create_game(%{name: "waiting game", edition: "webapp"})
+
+      assert {:ok, %Player{} = player} = Cornucopia.create_player(%{name: "Early Player", game_id: game.id})
+      assert player.name == "Early Player"
+      assert player.game_id == game.id
+    end
   end
 
   describe "cards" do

@@ -148,6 +148,10 @@ defmodule CopiWeb.GameLiveTest do
       assert html =~ game.name
     end
 
+    test "redirects to error when round parameter is invalid", %{conn: conn, game: game} do
+      assert {:error, {:redirect, %{to: "/error"}}} = live(conn, "/games/#{game.id}?round=abc")
+    end
+
     test "displays past round", %{conn: conn, game: game} do
        # Create players and play a round to make it valid
        {:ok, p1} = Cornucopia.create_player(%{name: "P1", game_id: game.id})

@@ -127,5 +127,39 @@ defmodule CopiWeb.PlayerLive.FormComponentTest do
 
       assert html =~ "can&#39;t be blank" or html =~ "blank"
     end
+
+    test "blocks player creation when game has already started", %{conn: conn, game: game} do
+      # Start the game
+      {:ok, _started_game} = Cornucopia.update_game(game, %{started_at: DateTime.truncate(DateTime.utc_now(), :second)})
+
+      # Try to navigate to the new player page - should redirect from mount
+      assert {:error, {:redirect, %{to: "/games"}}} = live(conn, "/games/#{game.id}/players/new")
+    end
+
+    test "blocks player creation via form submit when game started after page load", %{conn: conn, game: game} do
+      # Load the new player form while game is NOT started
+      {:ok, view, _html} = live(conn, "/games/#{game.id}/players/new")
+
+      # Start the game after the form is loaded (simulating race condition at the UI layer)
+      {:ok, _started_game} = Cornucopia.update_game(game, %{started_at: DateTime.truncate(DateTime.utc_now(), :second)})
+
+      # Submit the form - should be caught by form_component's Game.find check
+      view
+        |> form("#player-form", player: %{name: "Late Joiner", game_id: game.id})
+        |> render_submit()
+
+      # The view should have navigated away (push_navigate to /games/:id)
+      # The flash message should indicate the game has started
+      assert_redirect(view, "/games/#{game.id}")
+    end
+
+    test "shows error when trying to join non-existent game", %{conn: conn} do
+      fake_game_id = Ecto.ULID.generate()
+
+      # Trying to access a non-existent game's player page should raise/error
+      assert_raise Ecto.NoResultsError, fn ->
+        live(conn, "/games/#{fake_game_id}/players/new")
+      end
+    end
   end
 end
@@ -49,12 +49,24 @@ defmodule CopiWeb.PlayerLiveTest do
       assert html =~ "Cornucopia Web Session: some name"
     end
 
+    test "redirects on mount when game has already started", %{conn: conn, player: player} do
+      # Start the game
+      {:ok, started_game} = Cornucopia.update_game(
+        Cornucopia.get_game!(player.game_id),
+        %{started_at: DateTime.truncate(DateTime.utc_now(), :second)}
+      )
+
+      # Attempt to access the index page (which might mount the LiveView)
+      assert {:error, {:redirect, %{to: "/games"}}} = live(conn, "/games/#{started_game.id}/players")
+    end
+
     test "saves new player", %{conn: conn, player: player} do
       {:ok, index_live, _html} = live(conn, "/games/#{player.game_id}")
 
       assert index_live |> element(~s{[href="/games/#{player.game_id}/players/new"]}) |> render_click()
 
       assert_patch(index_live, "/games/#{player.game_id}/players/new")
+
       {:ok, index_live, _html} = live(conn, "/games/#{player.game_id}/players/new")
       assert index_live
              |> form("#player-form", player: @invalid_attrs)
@@ -70,6 +82,8 @@ defmodule CopiWeb.PlayerLiveTest do
       assert html =~ "Hi some updated name, waiting for the game to start..."
     end
 
+
+
     test "lists players on index route", %{conn: conn, player: player} do
       {:ok, _index_live, html} = live(conn, "/games/#{player.game_id}/players")
       assert html =~ "Listing Players"
@@ -153,13 +167,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()

@@ -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