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

Inconsistent Share Ownership When Depositing to a Different Receiver

eliab256

Root + Impact

Description

  • The BriVault::deposit function accepts two parameters: assets (the deposit amount) and receiver. The receiver parameter allows a user to deposit tokens while assigning the resulting vault shares to a different address.

  • The issue in the BriVault::deposit function occurs during the share minting process. Specifically, while the mapping BriVault::stakedAsset is correctly updated with the BriVault::receiver address, the minted shares are sent to msg.sender instead of the intended BriVault::receiver.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
​
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
​
uint256 fee = _getParticipationFee(assets);
// charge on a percentage basis points
if (minimumAmount + fee > assets) {
revert lowFeeAndAmount();
}
​
uint256 stakeAsset = assets - fee;
​
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);
​
​
emit deposited (receiver, stakeAsset);
​
return participantShares;
}

Risk

Likelihood:

  • This issue occurs every time a user calls BriVault::deposit with a BriVault::receiver address different from their own.
    When the BriVault::receiver is the same as msg.sender, the behavior is correct; however, if they differ, the minted shares are incorrectly assigned to msg.sender instead of the specified BriVault::receiver.

Impact:

  • This bug allows a receiver who does not actually hold any BriVault:shares to call the BriVault:joinEvent function, while preventing the user who originally called the BriVault:deposit function from participating — even though they do possess the BriVault:shares.

  • When BriVault:withdraw is executed, part of the funds become irretrievable, since these BriVault:shares are excluded from the reward distribution calculation, leading to a permanent loss of assets.

Proof of Concept

Add this code snippet to the provided test file.

This test verifies the mismatch between the shares actually held and the mapping that tracks staked assets. It also demonstrates how a user who possesses shares cannot participate due to this mismatch, while a user who does not actually hold any shares can participate even though they should not be able to.

function testDepositSendSharesToWrongAddress() public {
uint256 depositAmount = 1 ether;
uint256 expectedShares = briVault.convertToShares(depositAmount) * (10000 - participationFeeBsp) / 10000;
vm.startPrank(user1);
mockToken.approve(address(briVault), depositAmount);
briVault.deposit(depositAmount, user2);
vm.stopPrank();
// stakedAssetMapping should reflect user's shares amount
assertEq(briVault.balanceOf(user1), expectedShares); // user1 shares
assertEq(briVault.stakedAsset(user1), 0); // user1 mapping
assertEq(briVault.balanceOf(user2), 0); // user2 shares
assertEq(briVault.stakedAsset(user2), expectedShares ); // user2 mapping
​
//User2 with no shares can join event
vm.startPrank(user2);
briVault.joinEvent(0); // joining with countryId 0
vm.stopPrank();
​
//User1 with shares can't join event
vm.startPrank(user1);
vm.expectRevert(abi.encodeWithSelector(BriVault.noDeposit.selector));
briVault.joinEvent(0); // joining with countryId 0
vm.stopPrank();
​
}

Recommended Mitigation

To prevent this issue, the _to parameter in the BriVault::_mint call should be replaced with receiver instead of msg.sender.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
​
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
​
uint256 fee = _getParticipationFee(assets);
// charge on a percentage basis points
if (minimumAmount + fee > assets) {
revert lowFeeAndAmount();
}
​
uint256 stakeAsset = assets - fee;
​
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);
+ _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!