BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: high
Invalid

cancelParticipation() refunds the staked assets without the fee

0x_bob_0x

Root + Impact

Description

When users cancel, they receive their stakedAsset amount back, but the participation fee was already sent to participationFeeAddress during deposit. This means:

  • Users get back ONLY assets - fee, not their original assets

  • It contradicts user expectations if they think they can cancel "for free"

function deposit(uint256 assets, address receiver) public override returns (uint256) {
// ...
uint256 fee = _getParticipationFee(assets);
// ...
@> uint256 stakeAsset = assets - fee;
@> stakedAsset[receiver] = stakeAsset;
// ...
}
​
function cancelParticipation () public {
// ...
@> uint256 refundAmount = stakedAsset[msg.sender]; // Amount without the fee
stakedAsset[msg.sender] = 0;
uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares);
@> IERC20(asset()).safeTransfer(msg.sender, refundAmount); // Missing fee
}

Risk

Likelihood: High

  • Always when the user cancel before the tournament starts

Impact: Medium

  • Users permanently lose participation fee even if they cancel

  • Fee should either be refundable or documentation should be crystal clear

Proof of Concept

  • User deposited: 5 ETH

  • User received back: 4.925 ETH

  • User net loss: 0.075 ETH (PERMANENT)

function test_CancelParticipation_FeeLoss() public {
vm.startPrank(user1);
mockToken.approve(address(briVault), 10 ether);
​
uint256 depositAmount = 5 ether;
uint256 expectedFee = (depositAmount * briVault.participationFeeBsp()) / 10000;
uint256 expectedStake = depositAmount - expectedFee;
​
// Check balances before deposit
uint256 userBalanceBefore = mockToken.balanceOf(user1);
​
// User deposits
briVault.deposit(depositAmount, user1);
​
// User cancels participation
briVault.cancelParticipation();
​
// Check balances after cancel
uint256 userBalanceAfterCancel = mockToken.balanceOf(user1)
​
// Calculate total user loss
uint256 totalUserSpent = userBalanceBefore - userBalanceAfterCancel;
​
// User loses fee permanently
assertEq(totalUserSpent, expectedFee, "User permanently loses participation fee");
​
vm.stopPrank();
}

Recommended Mitigation

  1. Hold fee in the vault instead of sending immediately

  2. After tournament starts, transfer collected fees

+ mapping(address => uint256) public userFeesPaid;
+ uint256 public totalStaked;
​
function deposit(uint256 assets, address receiver) public override returns (uint256) {
// ... existing validation ...
​
uint256 fee = _getParticipationFee(assets);
uint256 stakeAsset = assets - fee;
​
stakedAsset[receiver] += stakeAsset;
+ userFeesPaid[receiver] += fee; // Track fee separately
+ totalStaked += stakeAsset;
​
- IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
- IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
// Hold fee in contract instead of sending immediately
+ IERC20(asset()).safeTransferFrom(msg.sender, address(this), assets);
​
_mint(msg.sender, participantShares);
}
​
function cancelParticipation() public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
uint256 refundAmount = stakedAsset[msg.sender];
+ uint256 feeRefund = userFeesPaid[msg.sender];
​
stakedAsset[msg.sender] = 0;
+ userFeesPaid[msg.sender] = 0;
+ totalStaked -= refundAmount;
​
uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares);
​
- IERC20(asset()).safeTransfer(msg.sender, refundAmount);
// Refund BOTH stake AND fee
+ IERC20(asset()).safeTransfer(msg.sender, refundAmount + feeRefund);
}
​
+function transferFeesToFeeAddress() external onlyOwner {
+ // After event starts, transfer collected fees
+ if (block.timestamp < eventStartDate){
+ revert eventNotStarted();
+ }
​
+ uint256 feesCollected = address(this).balance - totalStaked;
+ IERC20(asset()).safeTransfer(participationFeeAddress, feesCollected);
+}
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!