First Flight #9: Soulmate

Summary

The Staking contains issues in the deposit and withdraw functions where the comments suggest an increase or decrease in the userStakes variable, respectively. However, there is no validation for a zero amount deposit or withdrawal, which contradicts the logical comments. Additionally, the withdraw function lacks a check for whether the user has deposited before attempting to decrease the userStakes variable.

Vulnerability Details

The deposit function comment suggests increasing the userStakes variable, but it allows for zero amount deposits without any validation.

The withdraw function comment suggests decreasing the userStakes variable, but it allows for zero amount withdrawals without any validation.

The withdraw function also lacks a check to determine whether the user has deposited before attempting to decrease the userStakes variable.

Impact

1. Zero Amount Deposit:

   • Severity: Low

   • Consequence: Allowing zero amount deposits could lead to unexpected behavior and may contradict the intended logic of the deposit function.

2. Zero Amount Withdrawal:

   • Severity: Low

   • Consequence: Permitting zero amount withdrawals may result in unexpected outcomes and could conflict with the intended logic of the withdraw function.

3. Missing User Deposit Check:

   • Severity: Moderate

   • Consequence: The absence of a check to verify whether the user has deposited before withdrawal may lead to inconsistencies in the userStakes variable and unintended consequences.

POC

• Copy below test and run it via cmd forge test --match-test testUserDepositAndWithdrawlWithZeroAmount -vvvv

Result:

Recommendations

Deposit Function:

• Recommendation: Add a validation check to ensure that the amount is greater than zero before updating the userStakes variable in the deposit function.

• Example:

  require(amount > 0, "Amount must be greater than zero");

  userStakes[msg.sender] += amount;

Withdraw Function:

• Recommendation 1: Add a validation check to ensure that the amount is greater than zero before updating the userStakes variable in the withdraw function.

  • Example:

    require(amount > 0, "Amount must be greater than zero");

    userStakes[msg.sender] -= amount;

• Recommendation 2: Add a check to verify whether the user has deposited before proceeding with the withdrawal operation.

  • Example:

    require(userStakes[msg.sender] >= amount, "Insufficient funds");

    userStakes[msg.sender] -= amount;