First Flight #22: Steaking
First Flight #22
Beginner FriendlyFoundryDeFi
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

[H-03] Use raw_call instead of send low level function due to lack of gas sent

4rdiii

Summary

The unstake function in the Vyper smart contract uses the send method to transfer Ether to the specified address. However, send is subject to a 2300 gas limit which may not be sufficient for complex operations in the recipient contract. Additionally, there is no check to ensure that the send operation was successful, which could lead to Ether being sent unsuccessfully without any notification.

Vulnerability Details

In the unstake function:

@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) # No check if send is successful
​
log Unstaked(msg.sender, _amount, _to)

The send function is used to transfer Ether to the _to address. The send method forwards only 2300 gas, which may not be sufficient for contracts that require more gas to process received Ether. Moreover, if the send operation fails, it does not revert the transaction or provide any indication of failure, which could lead to loss of funds or incorrect contract behavior.

Impact

The current implementation can lead to failed Ether transfers when the recipient contract requires more than 2300 gas, causing potential loss of funds or incorrect state updates. The lack of failure handling in the send function can also result in funds being lost without any notification to the users or the contract.

Tools Used

Manual review

Recommendations

To address the issue, replace the send function with raw_call to gain more control over the gas limit and ensure successful Ether transfers. Additionally, include error handling to ensure that the transaction reverts if the Ether transfer fails.

Here is the revised unstake function using raw_call:

@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
​
success: bool = raw_call(
_to,
b"", # empty data, since we are just sending Ether
value=_amount,
gas=50000 # specify a custom gas limit
)
assert success, "Ether transfer failed"
​
log Unstaked(msg.sender, _amount, _to)

This modification uses raw_call to send Ether with a specified gas limit and checks if the transfer was successful, ensuring the contract can handle failures appropriately and avoid potential loss of funds.

Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year 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.