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

Unauthorized Withdrawal Due to Improper Deposit Ownership Validation in PerpetualVault.sol

silverwolf

Summary

The withdraw function in the smart contract fails to properly verify the ownership of a deposit, allowing a malicious actor to withdraw funds from another user’s deposit. This vulnerability arises from relying solely on EnumerableSet.contains(userDeposits[msg.sender], depositId), which can be manipulated in certain cases. By explicitly checking that the msg.sender is the original depositor recorded in depositInfo[depositId], the issue is mitigated.

Vulnerability Details

• Affected Function: withdraw(address recipient, uint256 depositId)

• Root Cause: The contract relies on EnumerableSet to verify deposit ownership instead of checking the actual depositor stored in depositInfo.

• Exploit Scenario:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "forge-std/Test.sol"; // Foundry for testing
import "../PerpetualVault.sol"; // Import the vulnerable contract
​
contract AttackPoC is Test {
VaultContract vault;
address alice = address(0x1);
address bob = address(this); // Attacker
​
function setUp() public {
vault = new VaultContract();
​
// Simulate Alice depositing 100 tokens
vault.deposit{value: 100}(alice, 100);
​
// Simulate Attacker manipulating the userDeposits mapping
vault.userDeposits(bob).add(1); // Deposit ID 1 belongs to Alice
}
​
function testAttack() public {
// Attacker withdraws Alice's deposit
vault.withdraw(bob, 1);
​
// Assert that the attacker now has Alice's funds
assertEq(vault.balanceOf(bob), 100);
}
}

1. Alice deposits funds.

2. Bob manipulates the contract (e.g., if there’s an unintended way to modify userDeposits to include an unauthorized deposit ID).

3. Bob calls withdraw() with Alice’s deposit ID and extracts her funds.

• Before Fix (Vulnerable Code):

if (EnumerableSet.contains(userDeposits[msg.sender], depositId) == false) {
revert Error.InvalidUser();
}

• Issue: This check only verifies if depositId exists in userDeposits[msg.sender], which could be modified or incorrectly updated in edge cases.
• After Fix (Safe Code):

if (depositInfo[depositId].depositor != msg.sender) {
revert Error.InvalidUser();
}

• Solution: This explicitly verifies that the caller (msg.sender) is the actual original depositor, preventing unauthorized withdrawals.

Impact

• Unauthorized users can withdraw funds from other users, leading to potential loss of assets.

• This vulnerability could be exploited in edge cases where deposit mappings are incorrectly modified (e.g., smart contract upgrades, mismanaged ownership transfers).

Tools Used

• Foundry (forge-std/Test.sol) for smart contract testing

• Manual Code Review to identify ownership validation issues

• PoC Attack Contract to simulate unauthorized withdrawal

Recommendations

• Explicitly check deposit ownership: Always verify that msg.sender matches the depositor field in depositInfo[depositId].

• Use structured access control: Implement access control mechanisms such as onlyDepositor modifiers.

• Audit state-changing functions: Ensure no other contract functions can manipulate userDeposits in unintended ways.

• Test edge cases: Simulate different attack vectors, including unauthorized deposit modifications, re-assignments, and governance actions.

Updates

Lead Judging Commences

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

Suppositions

There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.

Support

FAQs

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

Give us feedback!