Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: low
Invalid

Deposit and withdrawals validation checks missing

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

function testUserDepositAndWithdrawlWithZeroAmount() public {
uint amount = 0 ether;
loveToken.approve(address(stakingContract), amount);
console2.log("Before Deposit:", loveToken.balanceOf(address(stakingContract)));
stakingContract.deposit(amount);
console2.log("After Deposit:", loveToken.balanceOf(address(stakingContract)));
vm.prank(soulmate1);
console2.log("Balance of user after Withdrawal:", loveToken.balanceOf(soulmate1));
stakingContract.withdraw(amount);
console2.log("Balance of user after Withdrawal:", loveToken.balanceOf(soulmate1));
}

Result:

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.03ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

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;
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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