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

Incorrect Mint Recipient Mismatches Shares and Staked Assets, Leading to Loss of Funds

3mr_obito4

Root + Impact

Description

  • The deposit function is designed to allow one address (msg.sender) to deposit assets on behalf of another (receiver). The normal expectation is that the receiver is credited with the staked amount and is also issued the corresponding vault shares needed to withdraw their assets later.

  • The issue is that while the contract correctly records the staked asset amount for the receiver, it incorrectly mints the vault shares to the msg.sender. This creates a permanent desynchronization: the receiver has a stake recorded but holds no shares, while the msg.sender holds shares corresponding to an asset they did not contribute. As a result, the receiver can never withdraw their deposited funds.

// Root cause in the codebase with @> marks to highlight the relevant section
// File: src/briVault.sol
​
function deposit(
uint256 assets,
address receiver
) public override returns (uint256) {
require(receiver != address(0));
// ... fee calculation ...
uint256 stakeAsset = assets - fee;
​
@> stakedAsset[receiver] = stakeAsset; // <-- Correctly credits the receiver with the stake.
​
uint256 participantShares = _convertToShares(stakeAsset);
​
// ... fee and asset transfers ...
​
@> _mint(msg.sender, participantShares); // <-- MISTAKE: Mints shares to the sender, not the receiver.
​
emit deposited(receiver, stakeAsset);
​
return participantShares;
}

Risk

Likelihood:

  • The function signature deposit(uint256 assets, address receiver) explicitly encourages this flawed deposit flow.

  • Any scenario where msg.sender is different from receiver will trigger the bug.

Impact:

  • Guaranteed Loss of Funds for Receiver: The intended beneficiary of the deposit (receiver) receives no shares and therefore has no mechanism to ever access or withdraw their principal investment.

  • Broken Contract Invariants: The fundamental accounting principle that a user's stake should correspond to their share balance is violated, rendering the contract's logic untrustworthy.

Proof of Concept

This Proof of Concept test is specifically designed to demonstrate a critical flaw in the deposit function. It simulates a realistic scenario where one user (user1) deposits funds on behalf of another (user2). The test first confirms that the contract incorrectly gives ownership shares to the sender (user1) instead of the recipient (user2). It then proves that this mistake is fatal by having the rightful owner (user2) attempt to withdraw after winning, which causes the entire contract to crash with a division-by-zero error, permanently locking all funds.

function test_POC_IncorrectMintRecipientLocksFundsForReceiver() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user2);
vm.stopPrank();
​
uint256 expectedStake = (5 ether * (10000 - participationFeeBsp)) /
10000;
assertEq(
briVault.stakedAsset(user2),
expectedStake,
"User2 should have the staked asset"
);
​
assertEq(
briVault.balanceOf(user2),
0,
"BUG: User2 should have received shares, but has 0"
);
assertGt(
briVault.balanceOf(user1),
0,
"BUG: User1 should not have received shares"
);
​
vm.startPrank(user2);
briVault.joinEvent(10); // Bet on Japan
vm.stopPrank();
​
vm.warp(eventEndDate + 1 days);
vm.startPrank(owner);
briVault.setWinner(10); // Japan wins
vm.stopPrank();
​
uint256 user2BalanceBefore = mockToken.balanceOf(user2);
​
vm.startPrank(user2);
briVault.withdraw();
vm.stopPrank();
​
assertEq(
mockToken.balanceOf(user2),
user2BalanceBefore,
"User2 could not withdraw their funds"
);
}

Recommended Mitigation

  • dentifies the Target: The core of the change is focused on the _mint function call within the deposit logic.

  • Corrects the Recipient: It changes the address receiving the newly created shares from the transaction's sender (msg.sender) to the intended beneficiary (receiver).

  • Restores Logic: This simple, one-line change correctly links the deposited assets to the shares, ensuring the person who is credited with the stake is also the one who can control it.

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

Appeal created

bube 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!