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

The `joinEvent` function fails to prevent users from being repeatedly added to `usersAddress`, which can dilute the winners' withdrawal amounts.

minos

The joinEvent function fails to prevent users from being repeatedly added to usersAddress, which can dilute the winners' withdrawal amounts.

Description

  • When users call the joinEvent function to participate in the event, the protocol uses the usersAddress array to record participating users.

  • However, this function allows a single user to be repeatedly added to the usersAddress array, distorting the data in the usersAddress array.

  • Once usersAddress is used, the totalWinnerShares data becomes completely incorrect.

function joinEvent(uint256 countryId) public {
// ...Original code
@> usersAddress.push(msg.sender);
// ...Original code
}
function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
// ...Original code
@> _getWinnerShares();
// ...Original code
}
function _getWinnerShares () internal returns (uint256) {
@> for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId]; //
}
return totalWinnerShares;
}
function withdraw() external winnerSet {
// ...Original code
uint256 shares = balanceOf(msg.sender);
uint256 vaultAsset = finalizedVaultAsset;
@> uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);
_burn(msg.sender, shares);
@> IERC20(asset()).safeTransfer(msg.sender, assetToWithdraw);
emit Withdraw(msg.sender, assetToWithdraw);
}

Risk

Likelihood:

  • If a malicious actor correctly guesses the winnerCountryId, they can easily exploit this vulnerability.

Impact:

  • When any winner performs a withdraw, the funds they receive may be significantly less than they are entitled to.

Proof of Concept

  • Add the following function to test/BriVaultTest.t.sol and run forge test --mt test__joinEvent_withHugeUsers -vv

function test__joinEvent_AddUserRepeatedly() public {
// Generate a new user userX
address userX = makeAddr("userX");
mockToken.mint(userX, 1 ether);
// userX deposits all funds into the vault
vm.startPrank(userX);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, userX);
vm.stopPrank();
// Move timestamp to the block when the event starts
vm.warp(eventStartDate + 0);
// userX selects the country team with index 10
vm.startPrank(userX);
for(uint i=0; i<1000; i++) {
briVault.joinEvent(10);
}
vm.stopPrank();
// Move timestamp to after the event ends
vm.warp(eventEndDate + 1);
// Admin sets the winner
vm.prank(owner);
briVault.setWinner(10);
// userX withdraws as a winner
vm.prank(userX);
briVault.withdraw();
uint256 amountWinner = mockToken.balanceOf(userX);
console.log("amountWinner", amountWinner);
}

  • Console output:

Ran 1 test for test/briVault.t.sol:BriVaultTest
[PASS] test__joinEvent_AddUserRepeatedly() (gas: 29844436)
Logs:
amountWinner 985000000000000
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 23.19ms (21.07ms CPU time)

  • The above example shows a scenario where "only one user participates". They end up withdrawing 985000000000000 (i.e., 0.000985 ether).

  • If the maximum loop count in the test is changed to 1, they end up withdrawing 985000000000000000 (i.e., 0.985 ether).

  • The difference is significant.

Recommended Mitigation

  • In the joinEvent function, restrict the same user from "repeatedly executing the addition logic".

function joinEvent(uint256 countryId) public {
// Original code....
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
​
+ if (!userToJoinCountry[msg.sender]) {
usersAddress.push(msg.sender);
numberOfParticipants++;
totalParticipantShares += participantShares;
+ userToJoinCountry[msg.sender] = true;
+ }
​
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!