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

Shares Minted to `msg.sender` Instead of `receiver` in `BriVault::deposit` Causes Permanent Loss of Funds

fawarano

Shares Minted to msg.sender Instead of receiver in BriVault::deposit Causes Permanent Loss of Funds

Description

  • The deposit() function in BriVault is designed to accept deposits from msg.sender and allocate the corresponding staked assets and shares to a specified receiver address. This follows the ERC4626 standard where shares should be minted to the receiver parameter, allowing flexible deposit mechanisms such as depositing on behalf of another user.

  • However, the function mints shares to msg.sender instead of the receiver, while recording the staked assets under the receiver address. This creates a critical mismatch where the receiver has staked assets but zero shares, and msg.sender has shares but no staked assets. Neither party can effectively participate in the event or withdraw funds, resulting in permanent loss of deposited assets.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
​
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
​
uint256 fee = _getParticipationFee(assets);
if (minimumAmount + fee > assets) {
revert lowFeeAndAmount();
}
​
uint256 stakeAsset = assets - fee;
​
stakedAsset[receiver] = stakeAsset; // Staked assets recorded for receiver
​
uint256 participantShares = _convertToShares(stakeAsset);
​
IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
​
@> _mint(msg.sender, participantShares); // BUG: Minted to msg.sender instead of receiver
​
emit deposited(receiver, stakeAsset);
​
return participantShares;
}

Risk

Likelihood:

  • Any user depositing on behalf of another address triggers this vulnerability immediately upon calling deposit(amount, receiverAddress) where receiverAddress != msg.sender

  • Protocols integrating BriVault that implement deposit aggregators, vaults, or proxy contracts depositing on behalf of end users will experience this issue on every transaction

Impact:

  • Permanent loss of deposited funds, the receiver cannot withdraw winnings even when their selected team wins because they have 0 shares recorded in userSharesToCountry[receiver][countryId]

  • The msg.sender who receives the shares cannot join the event because BriVault::joinEvent requires stakedAsset[msg.sender] > 0 , causing revert with noDeposit() error

Proof of Concept

Place this test in test/briVault.t.sol and run

function test_deposit_receiver_shares_mismatch() public {
vm.prank(owner);
briVault.setCountry(countries);
​
address alice = user1;
address bob = user2;
uint256 depositAmount = 5 ether;
​
console.log("=== SCENARIO: Alice deposits for Bob ===");
​
// Alice deposits on behalf of Bob (receiver = Bob)
vm.startPrank(alice);
mockToken.approve(address(briVault), depositAmount);
uint256 shares = briVault.deposit(depositAmount, bob); // Bob is the receiver
vm.stopPrank();
​
console.log("\n=== AFTER DEPOSIT ===");
console.log("Shares returned:", shares);
console.log("Bob's stakedAsset:", briVault.stakedAsset(bob));
console.log("Alice's stakedAsset:", briVault.stakedAsset(alice));
console.log("Bob's share balance:", briVault.balanceOf(bob));
console.log("Alice's share balance:", briVault.balanceOf(alice));
​
// Verification: The bug is visible here
assertEq(briVault.stakedAsset(bob), 4.925 ether, "Bob should have stakedAssets");
assertEq(briVault.stakedAsset(alice), 0, "Alice should NOT have stakedAssets");
assertEq(briVault.balanceOf(alice), shares, "BUG: Alice has the shares");
assertEq(briVault.balanceOf(bob), 0, "BUG: Bob has ZERO shares");
​
console.log("\n=== Bob tries to joinEvent ===");
​
// Bob tries to join the event
vm.startPrank(bob);
briVault.joinEvent(10); // Bob chooses Japan (index 10)
vm.stopPrank();
​
// Check shares recorded for Bob in the mapping
uint256 bobSharesInCountry = briVault.balanceOf(bob);
console.log("Bob's shares recorded for country 10:", bobSharesInCountry);
​
// CRITICAL ISSUE: Bob has 0 shares recorded even though he has stakedAssets!
assertEq(bobSharesInCountry, 0, "CRITICAL BUG: Bob has 0 shares despite having stakedAssets");
​
console.log("\n=== Alice tries to joinEvent ===");
​
// Alice tries to join the event (she has shares but no stakedAsset)
vm.startPrank(alice);
vm.expectRevert(abi.encodeWithSignature("noDeposit()"));
briVault.joinEvent(20); // Alice tries to choose Spain (index 20)
vm.stopPrank();
​
console.log("Alice CANNOT join: she has shares but no stakedAsset");
​
// Owner set the index 10 as winner after Event ends
vm.warp(eventEndDate + 1);
vm.prank(owner);
briVault.setWinner(10);
​
vm.prank(bob);
vm.expectRevert(); // Reverts because shares = 0 used in withdraw calculation
briVault.withdraw();
console.log("Bob cannot withdraw even after winning because he doesn't have any shares");
}

Recommended Mitigation

Set the `receiver` to receive the participant shares instead of `msg.sender` :

function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
.
.
.
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
​
- _mint(msg.sender, participantShares);
+ _mint(receiver, participantShares);
​
emit deposited(receiver, stakeAsset);
​
return participantShares;
}
Updates

Appeal created

bryanconquer Lead Judge 19 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Shares Minted to msg.sender Instead of Specified Receiver

Support

FAQs

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

Give us feedback!