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

receiver/msg.sender Mismatch in deposit() bug.

0xsnoweth

Root + Impact

Description

  • Normal Behavior: When a user calls deposit(assets, receiver), the function should either:

    1. update stakedAsset[receiver] AND mint shares to receiver, OR

    2. update stakedAsset[msg.sender] AND mint shares to msg.sender. The accounting must be consistent.


  • Vulnerability: The deposit() function has a critical mismatch between who receives the accounting credit and who receives the shares. Line 264 updates stakedAsset[receiver] but line 278 mints shares to msg.sender. When receiver != msg.sender, this creates a complete accounting breakdown where:

  1. The receiver gets refund rights without owning shares

  2. The depositor gets shares without refund rights
    Both parties lose funds in different ways

function deposit(uint256 assets, address receiver) public override returns (uint256) {
// ... validation ...
uint256 stakeAsset = assets - fee;
@> stakedAsset[receiver] = stakeAsset; // Receiver gets refund rights
uint256 participantShares = _convertToShares(stakeAsset);
IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
@> _mint(msg.sender, participantShares); // msg.sender gets shares
emit deposited(receiver, stakeAsset);
return participantShares;
}

Risk

Likelihood: HIGH

  • Users naturally call deposit(amount, receiver) with different addresses for legitimate reasons (depositing on behalf of others, contract interactions, etc.)

  • The ERC4626 standard explicitly supports receiver != msg.sender as a core feature, so users expect this to work correctly
    No warnings or documentation indicate this parameter combination is dangerous

Impact: HIGH

  • Direct fund loss for depositor: Alice deposits 10 ETH but gets 0 refund when canceling, losing 9.85 ETH permanently
    Free money for receiver: Bob gets 9.85 ETH refund despite never depositing and having 0 shares
    Accounting corruption.

  • stakedAsset mapping becomes completely unreliable, breaking all refund calculations
    Protocol invariant violation: The fundamental invariant "users who deposit can cancel and get refunded" is broken

Proof of Concept

You may copy and paste this POC onto the existing test suite.

function test_recieverMsgSenderMismatchMismatchInDeposit() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
address alice = makeAddr("alice");
address bob = makeAddr("bob");
mockToken.mint(alice, 50 ether);
​
vm.startPrank(alice);
mockToken.approve(address(briVault), type(uint256).max);
​
//Alice calls deposit (10 ETH with the reciever being bob)
uint256 sharesOfAlice = briVault.deposit(10 ether, bob);
uint256 stakedAssetBob = briVault.stakedAsset(bob);
​
//2. Verify stakedAsset[bob] = 9.85 ETH but balanceOf(alice) = 9.85 shares
console.log("the staked asset of the reciever bob is this much: ", stakedAssetBob );
console.log("the amount of shares msg.sender alice currently has: ", sharesOfAlice );
vm.stopPrank();
​
​
//3. Bob calls cancelParticipation() → gets 9.85 ETH refund despite having 0 shares
vm.startPrank(bob);
uint256 stakedAssetOfBobBeforeCancel = briVault.stakedAsset(bob);
briVault.cancelParticipation();
uint256 stakedAssetOfBobAfterCancel = briVault.stakedAsset(bob);
vm.stopPrank();
​
console.log("bob's balance after refund amount = :", stakedAssetBob );
console.log("bob's total share amounts = 0"); //he did not deposit in the first place to have shares
​
​
vm.startPrank(alice);
briVault.cancelParticipation();
uint256 aliceRefundedAmount = briVault.stakedAsset(alice);
vm.stopPrank();
​
//4. Alice has shares but can't cancel (her stakedAsset[alice] = 0)
console.log("Alice existing shares = :",sharesOfAlice );
console.log("Alice refunded amount = :", aliceRefundedAmount);
assertTrue(aliceRefundedAmount < sharesOfAlice, "Alice must have 0 assets to refund while still having shares");
}

output:

Ran 1 test for test/briVault.t.sol:BriVaultTest
[PASS] test_recieverMsgSenderMismatchMismatchInDeposit() (gas: 1520487)
Logs:
the staked asset of the reciever bob is this much: 9850000000000000000
the amount of shares msg.sender alice currently has: 9850000000000000000
bob's balance after refund amount = : 9850000000000000000
bob's total share amounts = 0
Alice existing shares = : 9850000000000000000
Alice refunded amount = : 0
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.12ms (667.91µs CPU time)

The test output proves the vulnerability perfectly:

  1. Bob's staked asset: 9.85 ETH → gets full refund

  2. Bob's shares: 0 (never deposited himself)

  3. Alice's shares: 9.85 ETH worth

  4. Alice's refund: 0 ETH (her stakedAsset[alice] = 0)

Recommended Mitigation

Option 1: Make receiver and msg.sender consistent (recommended):

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;
+ stakedAsset[msg.sender] = 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);
+ emit deposited(msg.sender, stakeAsset);
return participantShares;
}

Option 2: Mint shares to receiver instead:

- _mint(msg.sender, participantShares);
+ _mint(receiver, 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!