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

Losing Shares On Direct Transfer in PerpetualVault

juggernaut63

Summary

In the deposit function in PerpetualVault users can make a deposit to get shares. However, if the user makes a direct transfer, the user will not get shares.

Vulnerability Details

// The deposit function is the only official way to get shares.
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);
// ...
}
​
// The _mint function is only called on authorized deposits
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;
// ...
}

The deposit function records the deposit information in the depositInfo mapping, adds the deposit ID to the userDeposits mapping, and grants shares via the _mint function. However, if someone transfers tokens directly to the contract (because the deposit function is marked as external payable), then the tokens will go to the contract but there is no record in depositInfo which means the user will not get shares.

Impact

Users who transfer directly will not get any shares.

POC

Add this to PerpetualVault.t.sol and run it forge test --match-test test_DirectTransferVulnerability --rpc-url arbitrum -vvvv.

function test_DirectTransferVulnerability() external {
// Setup initial state
address alice = makeAddr("alice");
uint256 directTransferAmount = 1000e6;
// Get the collateral token
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
// Give Alice some tokens for direct transfer
deal(address(collateralToken), alice, directTransferAmount);
// Record initial state
uint256 initialVaultBalance = collateralToken.balanceOf(vault);
uint256 initialTotalShares = PerpetualVault(vault).totalShares();
uint256[] memory initialDeposits = PerpetualVault(vault).getUserDeposits(alice);
// Alice directly transfers tokens to vault without using deposit()
vm.startPrank(alice);
collateralToken.transfer(vault, directTransferAmount);
vm.stopPrank();
// Verify tokens were transferred
assertEq(
collateralToken.balanceOf(vault),
initialVaultBalance + directTransferAmount,
"Vault balance should increase"
);
// Verify no shares were minted
assertEq(
PerpetualVault(vault).totalShares(),
initialTotalShares,
"Total shares should not change"
);
// Verify no deposit was recorded
uint256[] memory finalDeposits = PerpetualVault(vault).getUserDeposits(alice);
assertEq(
finalDeposits.length,
initialDeposits.length,
"No new deposits should be recorded"
);
// Verify tokens are effectively trapped
// The balance is in the vault but:
// 1. No shares were minted
// 2. No deposit info was recorded
// 3. No user deposits mapping was updated
assertTrue(
collateralToken.balanceOf(vault) > initialVaultBalance &&
PerpetualVault(vault).totalShares() == initialTotalShares &&
finalDeposits.length == initialDeposits.length,
"Tokens should be trapped without proper accounting"
);
}

Result:

├─ [27263] 0xaf88d065e77c8cC2239327C5EDb3A432268e5831::transfer(TransparentUpgradeableProxy: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], 1000000000 [1e9])
│ ├─ [26563] 0x86E721b43d4ECFa71119Dd38c0f938A75Fdb57B3::transfer(TransparentUpgradeableProxy: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], 1000000000 [1e9]) [delegatecall]
│ │ ├─ emit Transfer(from: alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6], to: TransparentUpgradeableProxy: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], value: 1000000000 [1e9])

Transfer of 1000 was successfully made from Alice to the vault.

├─ [1250] 0xaf88d065e77c8cC2239327C5EDb3A432268e5831::balanceOf(TransparentUpgradeableProxy: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211]) [staticcall]
│ │ └─ ← [Return] 1000000000 [1e9]

Balance vault now shows 1000000000.

├─ [1075] TransparentUpgradeableProxy::fallback() [staticcall]
│ ├─ [450] PerpetualVault::totalShares() [delegatecall]
│ │ └─ ← [Return] 0

Total shares remain 0 even though there are tokens in the vault.

├─ [1672] TransparentUpgradeableProxy::fallback(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6]) [staticcall]
│ ├─ [1041] PerpetualVault::getUserDeposits(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6]) [delegatecall]
│ │ └─ ← [Return] []

The deposits array for Alice is empty, indicating no deposits were recorded.

Tools Used

  • Manual review

  • Foundry

Recommendations

Only accept deposits via the proper deposit function.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Users mistake, only impacting themselves.

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Users mistake, only impacting themselves.

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

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

Give us feedback!