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

deposit receiver mismatch

jericho

Root + Impact

Description

  • The deposit function uses a receiver parameter that allows any user to deposit assets on behalf of another address, but mints the corresponding shares to the message sender (msg.sender). This creates a fundamental mismatch between asset ownership and share ownership, breaking the core accounting logic of the vault.

// Root cause in the codebase with @> marks to highlight the relevant section
function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
// ... validation logic ...
uint256 stakeAsset = assets - fee;
stakedAsset[receiver] = stakeAsset; // @> Assets recorded to receiver
uint256 participantShares = _convertToShares(stakeAsset);
_mint(msg.sender, participantShares); // @> But shares minted to msg.sender
return participantShares;
}

Risk

Likelihood:

  • The function signature with receiver parameter is inherited from ERC4626 standard but implemented incorrectly

  • Any user can accidentally or maliciously deposit for other addresses

  • The mismatch is not immediately visible but causes critical failures in downstream functions

  • This violates the principle of least surprise and breaks expected token accounting

Impact:

  • Broken accounting - stakedAsset mapping points to receiver but shares are owned by msg.sender

  • Failed withdrawals - withdraw() uses msg.sender to check country eligibility but the shares and asset records are mismatched

  • Locked funds - users may deposit for others unintentionally, locking both assets and shares

  • Contract state corruption - the internal accounting becomes inconsistent and unrecoverable

Proof of Concept

User1 deposits for User2,check if user2 has zero shares.

function test_depositReceiverMismatch() public {
vm.prank(owner);
briVault.setCountry(countries);
​
// User1 deposits for User2 - creating mismatch
vm.startPrank(user1);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user2); // Deposits FOR user2, but shares go TO user1
vm.stopPrank();
​
// User2 tries to join event - succeeds because stakedAsset[user2] is set
vm.prank(user2);
briVault.joinEvent(1);
​
// But user2 has zero shares, so their participation has no weight
assertEq(briVault.balanceOf(user2), 0);
​
// Event ends, country 1 wins
vm.warp(eventEndDate + 1);
vm.prank(owner);
briVault.setWinner(1);
​
// User2 tries to withdraw - fails because they have no shares
vm.prank(user2);
vm.expectRevert(); // Will revert due to zero shares or other checks
briVault.withdraw();
​
// User1 has shares but didn't join any country, so they can't withdraw either
vm.prank(user1);
vm.expectRevert();
briVault.withdraw();
​
// Result: Both assets and shares are permanently locked
}

Recommended Mitigation

Remove the receiver parameter and enforce that deposits can only be made for the message sender:

@@ -228,7 +238,7 @@ contract BriVault is ERC4626, Ownable {
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
- _mint(msg.sender, participantShares);
+ _mint(receiver, participantShares);
emit deposited (receiver, stakeAsset);
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!