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

Theft through cancelParticipation Logic

kanyoro

Root + Impact

The cancelParticipation function refunds the user's original stakedAsset amount, not the proportional value of their shares. An attacker can donate assets to the vault to inflate the share value, then wait for a user to cancel, effectively acquiring the appreciated value from them.

Description

  • Normal Behavior: A user who calls cancelParticipation function expects to get their deposited funds back less the fee they already paid.

  • The Problem: The cancelParticipation function burns the user's shares but only refunds them their original stakedAsset amount. If the vault's assets have increased, the shares are worth more than the original stake so the user is refunded but loses all the appreciated value.

// Root cause in src/briVault.sol
function cancelParticipation () public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
​
@> uint256 refundAmount = stakedAsset[msg.sender];
​
stakedAsset[msg.sender] = 0;
​
uint256 shares = balanceOf(msg.sender);
@> _burn(msg.sender, shares);
​
@> IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}

Risk

Likelihood: High

  • Reason 1: The attack is simple and can be executed by anyone.

  • Reason 2: The attacker would call transfer to donate assets and then simply waits for any user to call cancelParticipation

Impact: High

  • Impact 1: Direct loss of funds for any user who calls cancelParticipation

  • Impact 2: The value not sent to the user is left orphaned in the vault, which the attacker can then claim by using the inflation attack

Proof of Concept

This test shows a victim's shares appreciating after a donation, but cancelParticipation only refunds them their original stake.

function test_cancelParticipation_Steals_Value() public {
address attacker = makeAddr("attacker");
address victim = makeAddr("victim");
​
mockToken.mint(attacker, 50 ether);
mockToken.mint(victim, 10 ether);
​
// Victim deposits 10 ether, fee is 0.15 eth.
// Victim's stake is 9.85 ether.
vm.startPrank(victim);
mockToken.approve(address(briVault), 10 ether);
uint256 stakedAmount = briVault.stakedAsset(victim);
assertEq(stakedAmount, 9.85 ether);
uint256 victimBalanceBefore = mockToken.balanceOf(victim);
vm.stopPrank();
​
// Attacker donates 50 ether to the vault.
// The victim's 9.85e18 shares are now worth 9.85 + 50 ether.
vm.startPrank(attacker);
mockToken.transfer(address(briVault), 50 ether);
vm.stopPrank();
​
// Victim cancels participation
vm.startPrank(victim);
briVault.cancelParticipation();
uint256 victimBalanceAfter = mockToken.balanceOf(victim);
​
assertEq(victimBalanceAfter, victimBalanceBefore + stakedAmount); // Refunded 9.85 eth
assertEq(briVault.balanceOf(victim), 0); // Shares are burned
​
// The 50 ether donation is now orphaned in the vault
assertEq(mockToken.balanceOf(address(briVault)), 50 ether);
assertEq(briVault.totalSupply(), 0);
}

Recommended Mitigation

The function cancelParticipation should be removed and a user wishing to exit their position before the event starts should use the standard redeem or withdraw functions, which would correctly calculate their proportional share of the total assets.

// src/briVault.sol
@> - /**
@> - @dev cancel participation
@> - */
@> - function cancelParticipation () public {
@> - if (block.timestamp >= eventStartDate){
@> - revert eventStarted();
@> - }
@> -
@> - uint256 refundAmount = stakedAsset[msg.sender];
@> -
@> - stakedAsset[msg.sender] = 0;
@> -
@> - uint256 shares = balanceOf(msg.sender);
@> - _burn(msg.sender, shares);
@> -
@> - IERC20(asset()).safeTransfer(msg.sender, refundAmount);
@> - }
@> -
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!