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

Deposit mints shares to msg.sender but credits stakedAsset to receiver — ownership/accounting mismatch

ishaanbansal2312

Root + Impact

Description

NORMAL BEHAVIOUR:

  • When a user deposits assets into the vault, the same address should both be credited internally (e.g., stakedAsset[address]) and receive the minted vault shares. Invariant: ownerOfShares == ownerOfStakedAsset.

PROBLEM :

  • deposit() stores the deposited amount under stakedAsset[receiver] but mints the ERC20 vault shares to msg.sender. This creates a divergence between who is credited in the contract state and who actually holds the shares, breaking assumptions used by joinEvent, withdraw, cancelParticipation and payout logic.

// @> Root cause: stakedAsset is credited to `receiver`...
stakedAsset[receiver] = stakeAsset;
​
// ... but shares are minted to msg.sender (different actor).
// @> This creates ownership/accounting mismatch.
_mint(msg.sender, participantShares);
​

Risk

Likelihood:

  • Any caller can deliberately pass a different receiver than msg.sender — the mismatch is trivial to trigger.

  • No checks exist to enforce that receiver == msg.sender or that stakedAsset owner and minted share owner are the same.

Impact:

  • An attacker can deposit tokens for victimAddress (crediting stakedAsset[victim]) while minting shares to themselves — attacker gets economic rights (shares) while victim is credited for stake (can cause refund confusion).

  • joinEvent / winner eligibility uses stakedAsset and userToCountry while payouts use balanceOf (shares). Divergence allows attacker to withdraw or deny rightful payouts to others.

  • Refund logic (cancelParticipation) can refund to a user who does not own shares or assets, enabling theft or grief.

Proof of Concept

​
contract PoC {
BriVault public vault;
IERC20 public token;
constructor(BriVault _vault, IERC20 _token) {
vault = _vault;
token = _token;
}
​
function run(address victim) external {
// assume token approved and attacker has funds
// deposit for victim but msg.sender = attacker
vault.deposit(100, victim);
// After this, vault.stakedAsset[victim] > 0, vault.balanceOf(attacker) increased.
// Attacker may then transfer shares or later withdraw (depending on other flows).
}
}
​

Recommended Mitigation

Ensure the same address receives both the internal credit and minted shares. Either remove receiver parameter altogether, or if depositing on behalf is intended, mint to receiver and record stakedAsset[receiver]. Additionally, use actualReceived when computing shares (to prevent other separate share-inflation issues).

- // store staking info for the receiver
- 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);
+ // store staking info for the receiver and mint shares to the same address
+ // (Also consider computing shares from actualReceived after transfer — see improved pattern)
+ stakedAsset[receiver] = stakeAsset;
+ // transfer fee and stake
+ IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
+ IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
+ // compute participantShares from actual received (recommended)
+ uint256 balanceBefore = IERC20(asset()).balanceOf(address(this)) - stakeAsset; // optional capture point
+ uint256 participantShares = _convertToShares(stakeAsset); // or compute from actualReceived
+ _mint(receiver, participantShares);
​
Updates

Appeal created

bryanconquer Lead Judge 20 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!