consuldemocracy
consuldemocracy copied to clipboard
Race Condition due to lack of UNIQUE constraint on "votes" table
By default, the tuple (user_id, proposal_id) is not UNIQUE on votes table. This makes ActAsVotable vulnerable to race condition, where the same user can vote twice (or more) the same proposal.
ActAsVotable code:
votes = find_votes_by(options[:voter], options[:vote_scope])
if votes.count == (0) || options[:duplicate]
# this voter has never voted
vote = ActsAsVotable::Vote.new(
votable: self,
voter: options[:voter],
vote_scope: options[:vote_scope]
)
else
# this voter is potentially changing his vote
vote = votes.last
A thread can request database if a voter have already voted a proposal while the first thread have not executed insert statement yet leading to a race condition where the database responds votes.count = 0, the second thread will then enter into the condition and insert a second register because the database definition permits this behavior by default.
A possible plan for fixing this issue:
- Avoid duplicate votes in the future, maybe by locking
current_user
every time we usevote_by
(note: I'm not sure this is the best option) - Add a task removing existing duplicate votes
- Release a version of CONSUL so duplicate votes are removed before the unique index is added
- Add a unique index to the votes table or upgrade to a version of acts_as_votable implementing this restriction (see ryanto/acts_as_votable#211)
- Remove the
current_user
locks in the code (we might also look for ways to recover from aRecordNotUnique
exception :thinking:) - Release another version of CONSUL