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

Inadequate gas allocation in `unstake` function thus causing potential transaction failures and funds lockup.

Description:

The unstake function in the contract currently uses the send function to transfer ether to a specified address. The send function in Vyper is inherently limited as it only forwards a fixed gas stipend of 2300 gas to the recipient. This amount of gas is typically insufficient for anything beyond emitting an event in the recipient's contract. Additionally, the send function does not revert if the transfer fails; instead, it returns a boolean indicating success or failure. This makes error handling cumbersome and prone to being overlooked.

@external
def unstake(_amount: uint256, _to: address):
"""
@notice Allows users to unstake their staked ETH before the staking period ends. Users
can adjust their staking amounts to their liking.
@param _amount The amount of staked ETH to withdraw.
@param _to The address to send the withdrawn ETH to.
"""
assert not self._hasStakingPeriodEnded(), STEAK__STAKING_PERIOD_ENDED
assert _to != ADDRESS_ZERO, STEAK__ADDRESS_ZERO
stakedAmount: uint256 = self.usersToStakes[msg.sender]
assert stakedAmount > 0 and _amount > 0, STEAK__AMOUNT_ZERO
assert _amount <= stakedAmount, STEAK__INSUFFICIENT_STAKE_AMOUNT
self.usersToStakes[msg.sender] -= _amount
self.totalAmountStaked -= _amount
@> send(_to, _amount)
log Unstaked(msg.sender, _amount, _to)

Impact:

The improper use of the send function could lead to failed ether transfers when unstaking, particularly if the recipient's contract or address requires more than 2300 gas to execute its operations. Since the send function does not revert on failure, the contract could enter an inconsistent state where the ether remains locked within the contract, leading to potential loss of funds or failed user transactions.

Proof of Concept:

If the _to parameter in the send function requires more than 2300 gas, the send function will fail but not revert the transaction, thus leaving the contract in an unintended state.

Recommended Mitigation:

Replace the send function with the raw_call function, which allows for greater flexibility in gas allocation, thereby reducing the risk of transaction failures and ensuring the safe transfer of funds. You can read the documentation for more info. Vyper Docs

def unstake(_amount: uint256, _to: address):
"""
@notice Allows users to unstake their staked ETH before the staking period ends. Users
can adjust their staking amounts to their liking.
@param _amount The amount of staked ETH to withdraw.
@param _to The address to send the withdrawn ETH to.
"""
assert not self._hasStakingPeriodEnded(), STEAK__STAKING_PERIOD_ENDED
assert _to != ADDRESS_ZERO, STEAK__ADDRESS_ZERO
stakedAmount: uint256 = self.usersToStakes[msg.sender]
assert stakedAmount > 0 and _amount > 0, STEAK__AMOUNT_ZERO
assert _amount <= stakedAmount, STEAK__INSUFFICIENT_STAKE_AMOUNT
self.usersToStakes[msg.sender] -= _amount
self.totalAmountStaked -= _amount
- send(_to, _amount)
+ success: bool = False
+ response: Bytes[32] = b""
+ x: uint256 = _amount
+ success, response = raw_call(
+ _to,
+ abi_encode(x, method_id=method_id("unstake(uint256,address)")),
+ max_outsize=32,
+ value=_amount,
+ revert_on_failure=False
+ )
+ assert success
log Unstaked(msg.sender, _amount, _to)
Updates

Lead Judging Commences

inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

Usage of send is not the best thing

Support

FAQs

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