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

`deposit` mints participant shares to `msg.sender` instead of `receiver`

darkfox

Description

The protocol intends for the receiver specified in a deposit call to receive credit for the staked assets and corresponding participant shares of the vault.

However, the current implementation mints participant shares to msg.sender rather than the receiver. As a result, if the caller (msg.sender) and the receiver are different addresses, the shares will be issued to the wrong account.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
..
​
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;
}
​
function joinEvent(uint256 countryId) public {
@> if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
​
...
}

Risk

Likelihood:

This issue will occur whenever a user calls deposit on behalf of another address (when receiver != msg.sender). The effect only becomes critical if the participant shares are not transferred to the receiver before they attempt to call joinEvent.

Impact:

The receiver does not receive the participant shares, but they can still call joinEvent because stakedAsset[receiver] is populated. However, their team’s shares will not be reflected in userSharesToCountry. If the receiver’s team later wins, the totalWinningShares will be undercounted.

Moreover, since the receiver lacks the actual participant shares, they cannot redeem their winnings unless those shares are manually transferred from the original depositor. If this transfer occurs after joinEvent, share accounting will remain incorrect, allowing some winning users to withdraw a disproportionate amount of vault assets while other winning users will be prevented from withdrawing.

Proof of Concept

Add this test to the test suite in test/briVault.t.sol.

function testParticipationSharesGoToSenderRatherThanReceiverInDeposit() public {
vm.prank(owner);
briVault.setCountry(countries);
​
uint256 base = 10000;
uint256 depositAmount = 5e18;
uint256 feeAmount = (depositAmount * participationFeeBsp) / base;
​
vm.startPrank(user1);
mockToken.approve(address(briVault), depositAmount);
briVault.deposit(depositAmount, user2);
vm.expectRevert(BriVault.noDeposit.selector);
briVault.joinEvent(0);
vm.stopPrank();
​
vm.prank(user2);
briVault.joinEvent(0);
​
vm.startPrank(user3);
mockToken.approve(address(briVault), depositAmount);
briVault.deposit(depositAmount, user3);
briVault.joinEvent(0);
vm.stopPrank();
​
vm.warp(eventEndDate + 1);
​
vm.prank(owner);
briVault.setWinner(0);
​
uint256 user2BalanceBeforeWithdraw = mockToken.balanceOf(user2);
vm.prank(user2);
briVault.withdraw();
​
assert(user2BalanceBeforeWithdraw == mockToken.balanceOf(user2));
​
vm.expectRevert(BriVault.didNotWin.selector);
vm.startPrank(user1);
briVault.withdraw();
IERC20(briVault).transfer(user2, briVault.balanceOf(user1));
vm.stopPrank();
vm.prank(user2);
briVault.withdraw();
​
assert(mockToken.balanceOf(user2) == user2BalanceBeforeWithdraw + 2*(depositAmount - feeAmount));
}

This test demonstrates that when user1 deposits on behalf of user2, the shares are minted to user1 instead of user2. This makes it so that user2 can still join the event but initially cannot withdraw winnings because they do not hold the shares. After user1 transfers the shares to user2, user2 can successfully withdraw, but the vault’s internal accounting will be incorrect, leading to wrong share calculations.

Recommended Mitigation

Mint participant shares to the receiver address in deposit.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
..
​
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 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!