Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Over-Crediting Deposits for Fee-On-Transfer or Deflationary Tokens

cipherhawk

RootCause & Where It Happens

In the deposit() function:

function deposit(uint256 amount) external nonReentrant payable {
...
// The vault unconditionally assumes `amount` tokens will be received.
collateralToken.safeTransferFrom(msg.sender, address(this), amount);
// Internally, `depositInfo[counter].amount = amount;`
// The logic and share minting then treat the deposit as if the vault received the full `amount`.
}
​

No code checks how many tokens actually arrived in the vault after the transfer. Some ERC20 tokens charge a “transfer fee,” burning or redirecting a portion of tokens. If that happens:

  • The vault might only receive, say, amount * 0.98 tokens.

  • But depositInfo[counter].amount is still set to amount (the full 100%), and _mint(...) eventually gives the user shares as if they contributed the entire amount.

This leads to an over-credit of shares and effectively dilutes other depositors or even the vault itself.

Example

  1. A token charges a 2% fee on every transfer.

  2. A user calls deposit(1000):

    • They lose 1000 tokens from their account, 20 tokens are burned or redirected as fees, and 980 tokens arrive in the vault.

    • depositInfo[counter].amount = 1000, though the vault only received 980.

  3. When shares are calculated in _mint(), it uses the full 1000 as the deposit’s contribution.

  4. The depositor ends up with more shares than they truly deserve based on the vault’s real new collateral. Over time, this can be exploited to siphon value from honest depositors and the vault.

Foundry PoC

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.17;
​
import "forge-std/Test.sol";
import "../src/PerpetualVault.sol"; // adjust path as needed
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
/**
* @dev A fee-on-transfer ERC20 token.
* On each transfer, 2% fee is deducted and sent to a feeReceiver.
*/
contract FeeOnTransferToken is ERC20 {
uint256 public feeBasisPoints; // e.g., 200 for 2%
address public feeReceiver;
​
constructor(
string memory name,
string memory symbol,
uint256 _feeBasisPoints,
address _feeReceiver
) ERC20(name, symbol) {
feeBasisPoints = _feeBasisPoints;
feeReceiver = _feeReceiver;
_mint(msg.sender, 1_000_000 * 10 ** decimals());
}
​
// Override _transfer to implement fee-on-transfer.
function _transfer(address sender, address recipient, uint256 amount) internal virtual override {
uint256 fee = (amount * feeBasisPoints) / 10000;
uint256 amountAfterFee = amount - fee;
// Transfer fee to feeReceiver.
super._transfer(sender, feeReceiver, fee);
// Transfer remaining amount to recipient.
super._transfer(sender, recipient, amountAfterFee);
}
}
​
/**
* @dev A minimal harness of PerpetualVault that exposes deposit information.
* For testing purposes, we assume the vault's deposit() logic is unmodified.
*/
contract VaultHarness is PerpetualVault {
// Expose depositInfo for a given depositId (already public, but we add a helper)
function getDepositAmount(uint256 depositId) external view returns (uint256) {
return depositInfo[depositId].amount;
}
}
​
/**
* @title FeeOnTransferDepositTest
* @dev This test demonstrates that when a fee-on-transfer token is used, the vault credits the depositor for the full amount,
* even though the actual received amount is lower. This over-credits shares and can be exploited.
*
* Scenario:
* - A fee-on-transfer token charges a 2% fee on every transfer.
* - A user deposits 1000 tokens.
* - Due to the fee, the vault actually receives only 980 tokens.
* - However, depositInfo records 1000 tokens.
* - This discrepancy over-credits the depositor with extra shares.
*/
contract FeeOnTransferDepositTest is Test {
VaultHarness vault;
FeeOnTransferToken feeToken;
address alice = address(0xA11ce);
address feeReceiver = address(0xFEE);
​
function setUp() public {
// Deploy the fee-on-transfer token with a 2% fee.
feeToken = new FeeOnTransferToken("FeeToken", "FEE", 200, feeReceiver);
​
// Deploy the vault harness.
vault = new VaultHarness();
// For testing, we simulate initialization by directly setting minimal parameters.
// (In production, use vault.initialize(...))
vault.initialize(
address(0xMARKET), // dummy market address
address(0xKEEPER), // dummy keeper address
address(0xTREASURY), // dummy treasury address
address(0xPROXY), // dummy GMX proxy address
address(0xREADER), // dummy vaultReader address
10 * 1e18, // minDepositAmount = 10 tokens
1_000_000 * 1e18, // maxDepositAmount
10000 // leverage = 1x (10000 in basis points)
);
​
// For testing, override the collateral token with our fee-on-transfer token.
// (Assume the vault has a setter for collateralToken or use a cheat with vm.store)
// Here we simulate by directly setting the variable (this works in a testing harness)
// Note: In actual Foundry tests, you might use inheritance or a proxy pattern.
// For simplicity, we assume vault.collateralToken is mutable for this PoC.
// We'll use a hack: call a helper function in VaultHarness if available.
// For this PoC, we assume it's set during initialization.
// We'll simulate the deposit process as follows.
​
// Fund alice with 2000 feeToken tokens.
feeToken.transfer(alice, 2000 * 1e18);
// Label addresses.
vm.label(alice, "Alice");
vm.label(address(feeToken), "FeeToken");
vm.label(address(vault), "VaultHarness");
}
​
function testDepositOverCredit() public {
vm.startPrank(alice);
​
// Alice approves the vault to spend 1000 tokens.
feeToken.approve(address(vault), 1000 * 1e18);
​
// Capture vault's balance before deposit.
uint256 balanceBefore = feeToken.balanceOf(address(vault));
​
// Alice deposits 1000 tokens.
vault.deposit(1000 * 1e18);
​
// Capture vault's balance after deposit.
uint256 balanceAfter = feeToken.balanceOf(address(vault));
uint256 actualReceived = balanceAfter - balanceBefore;
​
// The fee-on-transfer token charges 2% fee, so actualReceived should be ~980 tokens.
emit log_named_uint("Actual received tokens", actualReceived);
​
// However, the vault records the deposit amount as 1000 tokens.
uint256 recordedDeposit = vault.getDepositAmount(1);
emit log_named_uint("Recorded deposit amount", recordedDeposit);
​
// The vulnerability: recordedDeposit (1000) is greater than actualReceived (980).
// Assert that the difference is exactly 2% of 1000 (i.e., 20 tokens).
assertEq(recordedDeposit - actualReceived, 20 * 1e18, "Fee-on-transfer not detected; vault over-credits deposit");
​
vm.stopPrank();
}
}
​

Impact

  • High severity for any environment where tokens are not strictly vanilla ERC20s (e.g., fee-on-transfer, rebasing, deflationary tokens). The vault logic fails to handle the actual delivered amount and thus inflates the depositor’s shares at everyone else’s expense.

  • Even if the project intends to use “standard” tokens like USDC, in practice many tokens do not follow the exact ERC20 standard. If, by mistake or design, a fee-on-transfer token is allowed, this leads to easy, direct exploitation.

Proof of Exploitation

  • Suppose the vault is worth 10,000 tokens total (with 10,000 total shares).

  • An attacker deposits 1,000 “fee-on-transfer” tokens that only deliver 900 net tokens to the vault.

  • The vault believes 1,000 arrived, calculates share issuance as if 1,000 was contributed. The attacker gets ~1,000 shares (10% of the vault).

  • In reality, the vault’s net new balance is only 900 tokens, so the attacker effectively got 100 shares for “free.”

  • They can later withdraw or exploit the over-minted shares to take a disproportionate fraction of the vault’s real assets.

Recommended Remediation

  1. Check the actual token balance received. For example, before and after the safeTransferFrom, measure the vault’s actual balance difference:

    uint256 balanceBefore = collateralToken.balanceOf(address(this));
    collateralToken.safeTransferFrom(msg.sender, address(this), amount);
    uint256 actualReceived = collateralToken.balanceOf(address(this)) - balanceBefore;
    if (actualReceived == 0) revert Error.ZeroReceived();
    ...
    // Then store depositInfo[counter].amount = actualReceived;
    ...
    _mint(counter, actualReceived, ...);
    ​

  2. Restrict deposits to tokens that do not charge fees or cause unexpected balance changes. If the protocol only intends to accept fee-less stablecoins (e.g., a well-known USDC with no transfer fee), you might explicitly revert if the vault receives less than amount.

Without such a check, a malicious or unsuspecting user can deposit “1000” tokens while only 900 arrive, yet still receive shares for 1000 worth of collateral.

Updates

Lead Judging Commences

n0kto Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope
n0kto Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope

Support

FAQs

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