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

Function 'withdraw' relies on current user's shares allowing to manipulate payout amounts

al88nsk

Description

Function withdraw relies on a user's shares when calculating assets amount to transfer:

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);
​
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);
}

This allows to manipulate assets amounts using transfer of BriVault tokens. For example, consider the scenario below:

  • User bets on all teams using different accounts and gets X shares for each account

  • After winner is set, the user transfers all shares to the winner account and gets TeamCount * X shares on the winner account

  • Now the user withdraws winnings and get much more than it must be

Risk

Likelihood:

High, since no requirements should be met

Impact:

High, since user gets more tokens than it must be

Recommended Mitigation

Fix shares that users have and use the fixed amount to calculate assets amount to transfer in withdraw

Updates

Appeal created

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