Multiple votes per user possible in the forum pallet via `vote_on_poll`

[mmostafas]

Summary

The vote_on_poll extrinsic in forum pallet does not check whether a user has already voted for a specific poll. This will enable a malicious forum user to submit an arbitrary number of votes and therefore manipulate the poll's result.

Issue Details

In runtime-modules/forum/src/lib.rs the vote_on_poll extrinsic can be used by forum users to vote on a specific PollAlternative by submitting it's index via this extrinsic. This extrinsic performs some validity checks via the ensure_vote_is_valid function, but this function only checks whether the poll exists and if it is not expired. Therefore a malicious forum user may submit arbitrary amount of votes and manipulate the poll's result.

    /// Check the vote is valid
    fn ensure_vote_is_valid(
        thread: Thread<ForumUserId<T>, T::CategoryId, T::Moment, T::Hash>,
        index: u32,
    ) -> Result<Poll<T::Moment, T::Hash>, Error<T>> {
        // Ensure poll exists
        let poll = thread.poll.ok_or(Error::<T>::PollNotExist)?;

        // Poll not expired
        if poll.end_time < <pallet_timestamp::Module<T>>::now() {
            Err(Error::<T>::PollCommitExpired)
        } else {
            let alternative_length = poll.poll_alternatives.len();
            // The selected alternative index is valid
            if index as usize >= alternative_length {
                Err(Error::<T>::PollData)
            } else {
                Ok(poll)
            }
        }
    }

Risk

A malicious forum user can submit an arbitrary number of votes and therefore manipulate a poll.

Mitigation

We suggest mitigating this by adding a tracking mechanism to identify which users have already voted. One possible implementation of this would be to add the voting users (forum_used_id) to the poll_alternatives field that is saved in storage. This way it can be checked whether forum_used_id already exists in any of the poll_alternatives before saving their vote. Furthermore, if the anonymity of the user's votes matter in this use case, the votes can be stored per "thread" instead of "alternatives".

Additionally, since this new implementation will store the votes on-chain, a deposit will also be required.