Potential Risk:
The _vote function in the CultureIndex contract takes two parameters: pieceId and voter. While the function includes some input validation checks, there are some potential issues and missing checks that should be addressed.
Proof of Concept (PoC):
Consider a scenario where an attacker attempts to vote for a piece with an invalid pieceId or an invalid voter address:
// Attempt to vote with an invalid pieceId
_vote(MAX_PIECE_ID + 1, msg.sender);
// Attempt to vote with an invalid voter address
_vote(pieceId, address(0));
In these PoC examples, an attacker tries to vote with an out-of-range pieceId and an invalid voter address. The _vote function should have additional checks to prevent such scenarios.
Recommended Mitigation Steps:
To mitigate the risk of using invalid pieceId or voter addresses, consider adding or enhancing input validation checks within the _vote function. Here's a recommended solution:
function _vote(uint256 pieceId, address voter) internal {
require(pieceId < _currentPieceId, "Invalid piece ID");
require(voter != address(0), "Invalid voter address");
require(!pieces[pieceId].isDropped, "Piece has already been dropped");
require(votes[pieceId][voter].voterAddress == address(0), "Already voted"); // Updated this check
require(_getPastVotes(voter, pieces[pieceId].creationBlock) > minVoteWeight, "Weight must be greater than minVoteWeight");
uint256 weight = _getPastVotes(voter, pieces[pieceId].creationBlock);
votes[pieceId][voter] = Vote(voter, weight);
totalVoteWeights[pieceId] += weight;
uint256 totalWeight = totalVoteWeights[pieceId];
// TODO add security consideration here based on block created to prevent flash attacks on drops?
maxHeap.updateValue(pieceId, totalWeight);
emit VoteCast(pieceId, voter, weight, totalWeight);
}
In this solution:
We've enhanced the input validation checks to ensure that the voter has not already voted for the same pieceId before allowing the vote.
We've also combined the check for the minimum weight requirement into a single require statement for efficiency.
These changes help prevent the use of invalid or unexpected input parameters and improve the overall input validation of the _vote function.
By implementing this solution, you can reduce the risk of issues related to invalid input parameters when voting for a piece.
Lines of code
https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/CultureIndex.sol#L307
Vulnerability details
Potential Risk: The
_vote
function in theCultureIndex
contract takes two parameters:pieceId
andvoter
. While the function includes some input validation checks, there are some potential issues and missing checks that should be addressed.Proof of Concept (PoC): Consider a scenario where an attacker attempts to vote for a piece with an invalid
pieceId
or an invalidvoter
address:// Attempt to vote with an invalid pieceId _vote(MAX_PIECE_ID + 1, msg.sender);
// Attempt to vote with an invalid voter address _vote(pieceId, address(0));
In these PoC examples, an attacker tries to vote with an out-of-range
pieceId
and an invalid voter address. The_vote
function should have additional checks to prevent such scenarios.Recommended Mitigation Steps: To mitigate the risk of using invalid
pieceId
orvoter
addresses, consider adding or enhancing input validation checks within the_vote
function. Here's a recommended solution:function _vote(uint256 pieceId, address voter) internal { require(pieceId < _currentPieceId, "Invalid piece ID"); require(voter != address(0), "Invalid voter address"); require(!pieces[pieceId].isDropped, "Piece has already been dropped"); require(votes[pieceId][voter].voterAddress == address(0), "Already voted"); // Updated this check require(_getPastVotes(voter, pieces[pieceId].creationBlock) > minVoteWeight, "Weight must be greater than minVoteWeight");
}
In this solution:
voter
has not already voted for the samepieceId
before allowing the vote._vote
function.By implementing this solution, you can reduce the risk of issues related to invalid input parameters when voting for a piece.
Assessed type
Invalid Validation