Liquid Staking

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

contracts/core/InsurancePool.sol

The provided Solidity code for the InsurancePool contract has several considerations for vulnerabilities and potential improvements. Here’s an analysis of the code:

1. Reentrancy Vulnerability

The withdraw function calls safeTransfer after burning tokens. If the safeTransfer triggers a fallback function that re-enters the contract, it can lead to unexpected behavior. To mitigate this risk:

  • Use the Checks-Effects-Interactions pattern:

    uint256 amountToTransfer = _amount; // Save amount to transfer before state changes
    _burn(msg.sender, amountToTransfer);
    totalDeposits -= amountToTransfer;
    token.safeTransfer(msg.sender, amountToTransfer);

2. Visibility of State Variables

Some state variables like totalDeposits and withdrawalRequests could be made private or internal, rather than public, if they're not intended to be accessed directly. This enhances encapsulation and reduces the attack surface.

3. Arithmetic Overflow/Underflow

While Solidity 0.8.x has built-in overflow and underflow checks, it’s good practice to ensure any mathematical operation (like totalDeposits += _amount;) does not lead to unexpected behavior, especially in complex calculations.

4. Claim Process Management

  • The current implementation allows the rebaseController to initiate claims but doesn’t restrict the withdrawal of tokens during the claim process effectively. It's necessary to ensure that during a claim process, no deposits or withdrawals can occur. Consider adding a modifier for that.

5. Access Control

The executeClaim function can be called only by the Owner. Ensure that the onlyOwner modifier is appropriately secured. If using OpenZeppelin's Ownable, it's crucial to manage the owner properly to prevent unwanted access. Consider using AccessControl if multiple roles are needed.

6. Withdrawal Window Logic

  • If a user requests a withdrawal and then attempts to deposit before the withdrawal is executed, they might unintentionally overwrite their withdrawal request. Consider adding checks or events to notify users of their current withdrawal state.

7. Max Claim Amount Validation

The validation in the executeClaim function uses a simple maximum cap based on the percentage of total deposits. Make sure the logic properly accounts for situations where the total deposits are very low or zero. This may lead to unexpected behavior.

8. Uninitialized State Variables

Ensure that any state variables that rely on other contracts (like rewardsPool) are properly initialized before use. For example, using rewardsPool before setting it could lead to unintended behavior.

9. Events Emission for State Changes

Consider emitting events for all significant state changes, especially in functions like deposit, withdraw, and requestWithdrawal, to ensure that all user actions are logged transparently.

10. Gas Limit and Loops

Although there are no loops in this code, if you extend this contract in the future, ensure that any loops (for example, processing multiple requests) do not exceed block gas limits.

Summary

While the code follows good practices, addressing these points can improve the security and maintainability of the InsurancePool contract. Make sure to thoroughly test the contract, including edge cases and potential attack vectors. Consider conducting an external audit for enhanced security, especially if it will manage significant amounts of funds.

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.