Race condition in voting system allows duplicate votes and data corruption

[immortal71]

Problem

The voting system in copi.owasp.org has a critical race condition that can lead to duplicate votes and data integrity issues. Both card voting and continue voting use a check-then-act pattern without proper database constraints or transaction isolation.

Root Cause

1. Missing Database Constraint on Card Votes

The votes table lacks a unique constraint on (player_id, dealt_card_id):

# priv/repo/migrations/20210816141921_create_votes.exs
create table(:votes) do
  add :player_id, references(:players, type: :uuid, on_delete: :nothing)
  add :dealt_card_id, references(:dealt_cards, on_delete: :nothing)
  timestamps()
end

create index(:votes, [:player_id])
create index(:votes, [:dealt_card_id])
# MISSING: unique constraint on [:player_id, :dealt_card_id]

2. Race Condition in Vote Toggle Logic

Both toggle_vote and toggle_continue_vote handlers in lib/copi_web/live/player_live/show.ex use unsafe check-then-act patterns:

Card Voting (lines 125-145):

vote = get_vote(dealt_card, player)

if vote do
  # Time window for race condition here
  Copi.Repo.delete!(vote)
else
  # Multiple concurrent requests can both pass this check
  case Copi.Repo.insert(%Copi.Cornucopia.Vote{
    dealt_card_id: String.to_integer(dealt_card_id), 
    player_id: player.id
  }) do
    {:ok, _vote} -> # ...
    {:error, _changeset} -> # ...
  end
end

Continue Voting (lines 102-116) has the same pattern.

Testing

Add concurrent voting test:

test "prevents duplicate votes from concurrent requests", %{game: game, player: player} do
  dealt_card = List.first(game.dealt_cards)
  
  # Simulate concurrent vote requests
  tasks = for _ <- 1..5 do
    Task.async(fn ->
      Repo.insert(%Vote{player_id: player.id, dealt_card_id: dealt_card.id})
    end)
  end
  
  results = Enum.map(tasks, &Task.await/1)
  successes = Enum.count(results, fn 
    {:ok, _} -> true
    _ -> false
  end)
  
  assert successes == 1, "Only one vote should succeed"
  assert Repo.aggregate(Vote, :count) == 1
end

Related Files

• lib/copi_web/live/player_live/show.ex (lines 95-150)
• priv/repo/migrations/20210816141921_create_votes.exs
• priv/repo/migrations/20260206100558_create_continue_votes.exs
• lib/copi/cornucopia/vote.ex
• lib/copi/cornucopia/continue_vote.ex

[immortal71]

@sydseter I think its good issue to work on !!

[sydseter]

@immortal71 sorry I didn’t assign you. As I mentioned, I want us to take our time with this. I’ll review carefully when I get the time.

[immortal71]

@sydseter okie no issue, take ur time on this

[sydseter]

The continue-vote portion is partially addressed because migration 20260206100558_create_continue_votes.exs adds a unique constraint on (player_id, game_id). However, the handler still uses check-then-act logic and inserts structs directly rather than handling the constraint through a changeset.