DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Invalid

totalDepositAmount Desynchronization from Contract Balance

Summary

The PerpetualVault.sol contract maintains a totalDepositAmount variable that tracks the total amount of ERC20 tokens deposited by users. However, inconsistencies can arise between this variable and the contract's actual token balance due to direct token transfers, potential execution failures, or logic errors in updating totalDepositAmount. Additionally, since totalDepositAmount cannot exceed maxDepositAmount, the contract balance should also never be greater than maxDepositAmount.

Vulnerability Details

Proof of code

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;

Impact

Issues

  1. Direct ERC20 Transfers: Users can send tokens directly to the contract without using deposit, causing the contract’s balance to increase while totalDepositAmount remains unchanged.

  2. Transfer Failures: If safeTransferFrom fails silently due to token implementation quirks, totalDepositAmount will be updated incorrectly.

  3. Incorrect totalDepositAmount Updates: If the transfer amount is lower than expected or fails mid-execution, totalDepositAmount will not reflect the correct balance.

  4. Exceeding maxDepositAmount: While totalDepositAmount cannot exceed maxDepositAmount, external transfers could still push the contract’s balance beyond maxDepositAmount, causing inconsistencies.

Proof of Concept (PoC)

Scenario 1: Direct Token Transfers
A user can send tokens directly to the contract, bypassing the deposit function:

IERC20(collateralToken).transfer(address(vaultContract), 100);

This increases the contract’s balance but does not update totalDepositAmount, leading to inconsistencies and potentially exceeding maxDepositAmount.

Scenario 2: ERC20 Token with Faulty safeTransferFrom Implementation

Some ERC20 tokens may have non-standard implementations that fail silently:

contract FaultyToken {
mapping(address => uint256) public balances;
mapping(address => mapping(address => uint256)) public allowance;
function transferFrom(address from, address to, uint256 amount) external {
if (balances[from] < amount) {
return; // Silent failure
}
balances[from] -= amount;
balances[to] += amount;
}
}

Tools Used

manual review

Recommendations

  1. Implement a Recovery Mechanism for Direct Transfers

Create a function to sync totalDepositAmount with the actual balance, ensuring it does not exceed maxDepositAmount:

function syncDepositAmount() external onlyOwner {
uint256 currentBalance = IERC20(collateralToken).balanceOf(address(this));
require(currentBalance <= maxDepositAmount, "Contract balance exceeds maxDepositAmount");
totalDepositAmount = currentBalance;
}
  1. Verify Transfer Success

Ensure that safeTransferFrom has executed successfully before updating totalDepositAmount:

function deposit(uint256 amount) external nonReentrant payable {
_noneFlow();
if (depositPaused) {
revert Error.Paused();
}
if (amount < minDepositAmount) {
revert Error.InsufficientAmount();
}
if (totalDepositAmount + amount > maxDepositAmount) {
revert Error.ExceedMaxDepositCap();
}
flow = FLOW.DEPOSIT;
uint256 balanceBefore = IERC20(collateralToken).balanceOf(address(this));
collateralToken.safeTransferFrom(msg.sender, address(this), amount);
uint256 balanceAfter = IERC20(collateralToken).balanceOf(address(this));
uint256 actualAmount = balanceAfter - balanceBefore;
if (actualAmount != amount) {
revert Error.TransferFailed();
}
require(balanceAfter <= maxDepositAmount, "Contract balance exceeds maxDepositAmount");
counter++;
depositInfo[counter] = DepositInfo(actualAmount, 0, msg.sender, 0, block.timestamp, address(0));
totalDepositAmount += actualAmount;
EnumerableSet.add(userDeposits[msg.sender], counter);
}

The issue of totalDepositAmount desynchronization is critical and could lead to financial loss or exploits. Additionally, ensuring that the contract balance never exceeds maxDepositAmount prevents unintended behaviors. Implementing the recommended mitigations will improve the contract’s reliability and security, ensuring accurate tracking of deposited funds.

Updates

Lead Judging Commences

n0kto Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
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.

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.

n0kto Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
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.

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.