BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Users Can Join Multiple Times Resulting in Unfair Winner Selection

nugen

[H-2] Users Can Join Multiple Times Resulting in Manipulated Winner Selection

Description:

  • The briVault.sol contains no method to check whether all users are unique or not which allow users to join multiple times

Risk

Likelihood:

  • As long a user has deposited, they can join as many times as they want

Impact:

  • The duplicateUser will have significantly big ratio to win the event.

  • The chance of winning for uniqueUsers will be much lower because they will get outnumbered by duplicateUsers.

Proof of Concept

  1. users make their deposit & join the event

  2. A malicious user joins multiple times which increases their chance of winning

// first, add this function into `briVault.sol`
function getUsersAddress() external view returns (address[] memory) {
return usersAddress;
}
​
// then run this test within `briVault.t.sol`
function test_users_can_join_multiple_times() public {
uint256 depositAmount = 0.1 ether;
​
vm.startPrank(user1);
mockToken.approve(address(briVault), depositAmount);
briVault.deposit(depositAmount, user1);
briVault.joinEvent(0);
​
vm.startPrank(user2);
mockToken.approve(address(briVault), depositAmount);
briVault.deposit(depositAmount, user2);
briVault.joinEvent(1);
​
vm.startPrank(user3);
mockToken.approve(address(briVault), depositAmount);
briVault.deposit(depositAmount, user3);
briVault.joinEvent(2);
​
// user2 joins again!
vm.startPrank(user2);
mockToken.approve(address(briVault), depositAmount);
briVault.deposit(depositAmount, user2);
briVault.joinEvent(1);
​
address[] memory users = briVault.getUsersAddress();
​
// assertion
bool hasDuplicateUser = false;
for (uint256 i = 0; i < users.length; i++) {
for (uint256 j = i + 1; j < users.length; j++) {
if (users[i] == users[j]) {
hasDuplicateUser = true;
}
}
}
assertEq(hasDuplicateUser, true);
}
​

Recommended Mitigation

  • It's best to use a mapping to track the users who joined the event.

  • Make the following mitigations within briVault.sol.

+error userAlreadyJoined();
+mapping(address => bool) public hasJoined;
​
function joinEvent(uint256 countryId) public {
+ require(!hasJoined[msg.sender], userAlreadyJoined());
​
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
​
// Ensure countryId is a valid index in the `teams` array
if (countryId >= teams.length) {
revert invalidCountry();
}
​
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
+ hasJoined[msg.sender] = true;
usersAddress.push(msg.sender);
​
numberOfParticipants++;
totalParticipantShares += participantShares;
​
emit joinedEvent(msg.sender, countryId);
}
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Duplicate registration through `joinEvent`

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!