Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: medium
Invalid

contracts/ethStaking/RewardsReceiver.sol

The Solidity code you provided implements a RewardsReceiver contract that manages the receipt and withdrawal of rewards for an ETH staking strategy. Below are some potential vulnerabilities and areas for improvement:

1. Reentrancy Vulnerability

While the contract uses the call method to transfer ETH, which is less prone to reentrancy issues than transfer or send, it does not have any reentrancy protection. If the ethStakingStrategy is a contract that calls back into withdraw, it could potentially lead to reentrancy attacks. To mitigate this, you can implement a reentrancy guard:

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
contract RewardsReceiver is Ownable, ReentrancyGuard {
// Rest of your code...
function withdraw() external nonReentrant returns (uint256) {
// Withdrawal logic...
}
}

2. Gas Limit Issues

Using the call method for transferring ETH can fail if the receiving contract does not have enough gas to execute its fallback function. This may result in failed transfers without a clear error. Ensure that the receiving contract can handle the transfer appropriately or implement a mechanism to check gas limits.

3. Owner Privileges

The setWithdrawalLimits function allows the owner to change the withdrawal limits. If the contract’s ownership is compromised, the attacker could set limits that prevent valid withdrawals. Consider implementing a multi-signature approach for sensitive actions, or a timelock mechanism to allow time for stakeholders to react to potentially malicious changes.

4. Immutable Variables

The ethStakingStrategy variable is marked as immutable, which is good as it prevents changes after deployment. However, if this address is compromised or incorrect at deployment, it cannot be changed later. Ensure that this address is thoroughly vetted before deployment.

5. Withdrawals Under Minimum Limit

In the withdraw function, if the balance is below minWithdrawalAmount, the function will return 0 without performing any checks or emitting an event. This could potentially lead to confusion. You might consider emitting an event in this case to clarify that no funds were withdrawn due to the balance being insufficient.

6. Visibility Modifiers

The withdraw function could be marked as external rather than the default public, which can save gas and improve clarity.

7. Event Emission for State Changes

Although you emit events for significant actions, consider emitting an event when withdrawal limits are set. This is already implemented, but it's crucial to ensure that all state-changing functions are logged appropriately for transparency.

8. No Fallback Function Handling

You have a receive function to accept ETH, but if someone sends ETH with additional data, it will be rejected. Depending on your application, you might want to implement a fallback function or ensure that users are aware they must send ETH without data.

9. Integer Overflow/Underflow (Pre-Solidity 0.8.0)

While Solidity 0.8.0 has built-in overflow and underflow checks, it's good to be aware of such issues in earlier versions. Ensure that all arithmetic operations, particularly when dealing with withdrawal amounts, are validated against potential overflows.

Conclusion

While the code looks generally sound, addressing these points will enhance its security and usability. Always conduct thorough testing and consider an audit by a trusted third party, especially for contracts handling ETH or other assets.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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