diff --git a/copi.owasp.org/lib/copi_web/live/player_live/show.ex b/copi.owasp.org/lib/copi_web/live/player_live/show.ex index bc71e3d8d..3f3b21c56 100644 --- a/copi.owasp.org/lib/copi_web/live/player_live/show.ex +++ b/copi.owasp.org/lib/copi_web/live/player_live/show.ex @@ -131,26 +131,32 @@ defmodule CopiWeb.PlayerLive.Show do {:ok, dealt_card} = DealtCard.find(dealt_card_id) - vote = get_vote(dealt_card, player) - - if vote do - Logger.debug("Player has voted: player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}") - Copi.Repo.delete!(vote) + # Critical authorization check: prevent self-voting + if dealt_card.player_id == player.id do + Logger.warning("Self-voting attempt blocked: player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}") + {:noreply, socket} else - Logger.debug("Player has not voted: player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}") - case Copi.Repo.insert(%Copi.Cornucopia.Vote{dealt_card_id: String.to_integer(dealt_card_id), player_id: player.id}) do - {:ok, _vote} -> - Logger.debug("Vote added successfully for player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}") - {:error, changeset} -> - Logger.warning("Voting failed for player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}, errors: #{inspect(changeset.errors)}") + vote = get_vote(dealt_card, player) + + if vote do + Logger.debug("Player has voted: player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}") + Copi.Repo.delete!(vote) + else + Logger.debug("Player has not voted: player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}") + case Copi.Repo.insert(%Copi.Cornucopia.Vote{dealt_card_id: String.to_integer(dealt_card_id), player_id: player.id}) do + {:ok, _vote} -> + Logger.debug("Vote added successfully for player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}") + {:error, changeset} -> + Logger.warning("Voting failed for player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}, errors: #{inspect(changeset.errors)}") + end end - end - {:ok, updated_game} = Game.find(game.id) + {:ok, updated_game} = Game.find(game.id) - CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game) + CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game) - {:noreply, assign(socket, :game, updated_game)} + {:noreply, assign(socket, :game, updated_game)} + end end def topic(game_id) do 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 5d17a5536..92ddc97e7 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 @@ -123,6 +123,44 @@ defmodule CopiWeb.PlayerLiveTest do assert Copi.Repo.get_by(Copi.Cornucopia.Vote, dealt_card_id: dealt.id, player_id: player.id) end + test "prevents self-voting authorization bypass", %{conn: conn, player: player} do + game_id = player.game_id + + # Create a card for the current player + {:ok, card} = Cornucopia.create_card(%{ + category: "C", value: "Self", description: "Self Card", edition: "webapp", + version: "2.2", external_id: "SELF", language: "en", + misc: "misc", owasp_scp: [], owasp_devguide: [], owasp_asvs: [], + owasp_appsensor: [], capec: [], safecode: [], owasp_mastg: [], owasp_masvs: [] + }) + {:ok, own_dealt_card} = Copi.Repo.insert(%Copi.Cornucopia.DealtCard{player_id: player.id, card_id: card.id, played_in_round: 1}) + + # Setup other player's card BEFORE mounting LiveView to ensure DOM contains vote buttons + {:ok, other_player} = Cornucopia.create_player(%{name: "Other", game_id: game_id}) + {:ok, other_card} = Cornucopia.create_card(%{ + category: "C", value: "Other", description: "Other Card", edition: "webapp", + version: "2.2", external_id: "OTHER", language: "en", + misc: "misc", owasp_scp: [], owasp_devguide: [], owasp_asvs: [], + owasp_appsensor: [], capec: [], safecode: [], owasp_mastg: [], owasp_masvs: [] + }) + {:ok, other_dealt_card} = Copi.Repo.insert(%Copi.Cornucopia.DealtCard{player_id: other_player.id, card_id: other_card.id, played_in_round: 1}) + + {:ok, show_live, _html} = live(conn, "/games/#{game_id}/players/#{player.id}") + + # Attempt to vote on own card (simulates JavaScript exploit by calling event directly) + # This should be blocked by the authorization check in handle_event + render_hook(show_live, "toggle_vote", %{"dealt_card_id" => "#{own_dealt_card.id}"}) + + # Verify NO vote was created - self-voting blocked + refute Copi.Repo.get_by(Copi.Cornucopia.Vote, dealt_card_id: own_dealt_card.id, player_id: player.id) + + # Vote on other player's card should work (DOM now contains this element) + show_live |> element("[phx-click=\"toggle_vote\"][phx-value-dealt_card_id=\"#{other_dealt_card.id}\"]") |> render_click() + + # Verify vote was created for other player's card + assert Copi.Repo.get_by(Copi.Cornucopia.Vote, dealt_card_id: other_dealt_card.id, player_id: player.id) + end + test "allows continue voting and resets votes on next round", %{conn: conn, player: player} do # Setup another player game_id = player.game_id