First Flight #9: Soulmate
First Flight #9
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

The staking contract is being used as token vault

ceseshi

Summary

When a user deposits tokens to stake, the tokens are not being sent to the staking vault, but to the staking contract.

Vulnerability Details

The staking contract has a token vault, which is a good security measure, as it provides additional security for the tokens and makes it easier to change the protocol without affecting the funds. But the staking contract is not sending the deposited tokens to the vault, but to the staking contract itself.

This test deposits 1000 tokens to the staking contract and checks the balance.

function testStakingVault() public {
// Give tokens to the user
deal(address(loveToken), soulmate1, 1000);
​
// Deposit to staking
vm.startPrank(soulmate1);
loveToken.approve(address(stakingContract), 1000);
stakingContract.deposit(1000);
​
// Check the staking contract balance
assertEq(loveToken.balanceOf(address(stakingContract)), 1000);
}

The test confirms that the tokens are deposited in the staking contract, not the vault.

Running 1 test for test/unit/AuditTest1.t.sol:AuditTest1
[PASS] testStakingVault() (gas: 213080)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.87ms

Impact

The staking contract is not using the vault, which is a security risk if the staking contract is compromised.

Tools Used

Foundry, Manual review

Recommendations

Change deposit() and withdraw() functions in Staking.sol to use the vault.

/// @notice Increase the userStakes variable and transfer LoveToken to this contract.
function deposit(uint256 amount) public {
if (loveToken.balanceOf(address(stakingVault)) == 0)
revert Staking__NoMoreRewards();
// No require needed because of overflow protection
userStakes[msg.sender] += amount;
- loveToken.transferFrom(msg.sender, address(this), amount);
+ loveToken.transferFrom(msg.sender, address(stakingVault), amount);
​
emit Deposited(msg.sender, amount);
}
​
/// @notice Decrease the userStakes variable and transfer LoveToken to the user withdrawing.
function withdraw(uint256 amount) public {
// No require needed because of overflow protection
userStakes[msg.sender] -= amount;
- loveToken.transferFrom(address(this), msg.sender, amount);
+ loveToken.transferFrom(address(stakingVault), msg.sender, amount);
emit Withdrew(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.