Summary
The function Steaking:stake should not allow users to send eth directly to the contract as the contract cant call unstake or depositIntoVault this ETH becomes permanently locked in the contract.
@external
@payable
def stake(_onBehalfOf: address):
"""
@notice Allows users to stake ETH for themselves or any other user within the staking period.
@param _onBehalfOf The address to stake on behalf of.
"""
assert not self._hasStakingPeriodEnded(), STEAK__STAKING_PERIOD_ENDED
assert msg.value >= MIN_STAKE_AMOUNT, STEAK__INSUFFICIENT_STAKE_AMOUNT
assert _onBehalfOf != ADDRESS_ZERO, STEAK__ADDRESS_ZERO
self.usersToStakes[_onBehalfOf] = msg.value
self.totalAmountStaked += msg.value
log Staked(msg.sender, msg.value, _onBehalfOf)
Vulnerability Details (POC)
add console to the test suite:
// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
- import {Test} from "lib/forge-std/src/Test.sol";
// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+ import {Test, console} from "lib/forge-std/src/Test.sol";
add the following test to the test suite:
function testUserCanSendEthToLockInContract() public {
uint256 dealAmount = 1 ether;
vm.deal(user1, dealAmount);
_stake(user1, dealAmount, address(steaking));
uint256 contractBalance = address(steaking).balance;
console.log("contract balance", contractBalance);
uint256 user1Balance = steaking.usersToStakes(user1);
console.log("user1 balance", user1Balance);
uint256 contractUserbalance = steaking.usersToStakes(address(steaking));
console.log("contract user balance", contractUserbalance);
}
the output shows:
Logs:
contract balance 1000000000000000000
user1 balance 0
contract user balance 1000000000000000000
Impact
Since the contract is not a user they cannot externally call unstake or depositIntoVault when the staking period ends. Therefore this ETH is permanently locked in the contract.
Tools Used
Foundry
Manual Review
Recommendations
Add an assert statement in the stake function that denys users from staking funds on behalf of the contract itself.
assert not self._hasStakingPeriodEnded(), STEAK__STAKING_PERIOD_ENDED
assert msg.value >= MIN_STAKE_AMOUNT, STEAK__INSUFFICIENT_STAKE_AMOUNT
assert _onBehalfOf != ADDRESS_ZERO, STEAK__ADDRESS_ZERO
+ assert _onBehalfOf != self, "Steak__CannotStakeOnBehalfOfContract"
