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

Support for Non-Standard ERC20 Assets (Fee-on-Transfer or Rebasing)

osamaebaid

Root + Impact

Description

  • The vault uses ERC20 asset transfers in deposit, withdraw, and cancelParticipation, assuming standard behavior where transferred amounts match exactly and balances don't change unexpectedly.

  • Share calculations in _convertToShares and payouts in withdraw rely on precise IERC20(asset()).balanceOf(address(this)) readings.

  • The issue is that if the asset is fee-on-transfer (e.g., deducts 1% on transfer) or rebasing (e.g., balances auto-adjust for yield), the vault receives less than expected in deposits or sees fluctuating balances, leading to over-minting shares or insufficient assets for withdrawals.

  • This causes incorrect share allocation, vault insolvency, or unfair dilution, as the contract doesn't account for or reject non-standard tokens.

// In deposit - assumes full transfer amount received
function deposit(uint256 assets, address receiver) public override returns (uint256) {
...
​
uint256 fee = _getParticipationFee(assets);
uint256 stakeAsset = assets - fee;
​
stakedAsset[receiver] = stakeAsset;
​
uint256 participantShares = _convertToShares(stakeAsset);
​
@> IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee); // @> If fee-on-transfer, less received but full calc used
​
@> IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset); // @> Same; rebasing changes balance post-transfer
​
_mint(msg.sender, participantShares);
​
...
}
​
// In _convertToShares - uses raw balance, vulnerable to rebasing
function _convertToShares(uint256 assets) internal view returns (uint256 shares) {
@> uint256 balanceOfVault = IERC20(asset()).balanceOf(address(this)); // @> Rebasing alters this unexpectedly
​
uint256 totalShares = totalSupply();
​
if (totalShares == 0 || balanceOfVault == 0) {
return assets;
}
​
shares = Math.mulDiv(assets, totalShares, balanceOfVault);
}

Risk

Likelihood: High

  • Deployer or users select a fee-on-transfer or rebasing ERC20 as the asset.

  • No token compatibility checks in constructor or functions.

Impact: High

  • Over-minted shares lead to insufficient assets on withdrawals, causing vault insolvency.

  • Users receive unfair or zero payouts, resulting in funds loss or dilution.

Proof of Concept

  • Add poc code as a separate test file

  • Run forge test --mt testFeeOnTransferOverMint

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import "forge-std/Test.sol";
import {BriVault} from "./BriVault.sol"; // Assume BriVault is in the same directory
​
contract FeeToken is ERC20 {
uint256 constant FEE_BPS = 100; // 1% fee on transfer
​
constructor() ERC20("FeeToken", "FEE") {}
​
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
​
function transferFrom(address from, address to, uint256 amount) public override returns (bool) {
uint256 fee = (amount * FEE_BPS) / 10000;
_burn(from, fee);
_transfer(from, to, amount - fee);
return true;
}
​
function transfer(address to, uint256 amount) public override returns (bool) {
uint256 fee = (amount * FEE_BPS) / 10000;
_burn(msg.sender, fee);
_transfer(msg.sender, to, amount - fee);
return true;
}
}
​
contract BriVaultPoCTest is Test {
BriVault vault;
FeeToken asset; // Fee-on-transfer token
address owner = address(this);
address user = address(0x123);
uint256 participationFeeBsp = 100; // 1%
uint256 eventStartDate = block.timestamp + 1 days;
uint256 eventEndDate = block.timestamp + 2 days;
uint256 minimumAmount = 100;
address participationFeeAddress = address(0xfee);
​
function setUp() public {
asset = new FeeToken();
vault = new BriVault(IERC20(address(asset)), participationFeeBsp, eventStartDate, participationFeeAddress, minimumAmount, eventEndDate);
​
string[48] memory countries;
for (uint i = 0; i < 48; i++) {
countries[i] = "Country";
}
vault.setCountry(countries);
​
asset.mint(user, 1e18 + 1e16); // Extra for fees
vm.prank(user);
asset.approve(address(vault), 1e18 + 1e16);
}
​
function testFeeOnTransferOverMint() public {
uint256 depositAmount = 1000;
uint256 expectedReceived = depositAmount * (10000 - 100) / 10000; // 1% less due to fee token
​
vm.prank(user);
uint256 shares = vault.deposit(depositAmount, user);
​
// Over-minted: shares based on full amount, but vault received less
uint256 actualBalance = IERC20(address(asset)).balanceOf(address(vault));
assertLt(actualBalance, depositAmount - (depositAmount * participationFeeBsp / 10000), "Vault received less due to fee token");
​
// Later withdraw would fail due to shortfall
}
}

Recommended Mitigation

  • Add token compatibility checks and virtual assets for rebasing

  • In constructor - revert if incompatible

  • For rebasing: use virtual offsets as in inflation mitigation

​
+ function isCompatibleToken(IERC20 token) internal view returns (bool) {
+ // Test transfer and balance consistency
+ return true; // Implement checks (e.g., transfer 1 wei, verify balance)
+ }
​
​
constructor (...) {
+ if (!isCompatibleToken(_asset)) revert("Incompatible asset");
​
...
}
​
​
+ uint256 internal constant VIRTUAL_ASSETS = 1;
+ uint256 internal constant VIRTUAL_SHARES = 1e18;
​
function totalAssets() public view override returns (uint256) {
- return IERC20(asset()).balanceOf(address(this));
+ return IERC20(asset()).balanceOf(address(this)) + VIRTUAL_ASSETS;
}
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Fee on transfer tokens

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!