Liquid Staking

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

contracts/core/sdlPool/SDLPoolPrimary.sol

Here are some potential vulnerabilities and areas for improvement to consider:

1. Reentrancy Attacks

While there are checks in place to ensure that certain functions cannot be called under specific conditions, the safeTransfer calls can still be susceptible to reentrancy attacks. To mitigate this risk, consider using the Checks-Effects-Interactions pattern. This means updating the state before transferring tokens.

2. Gas Limit Issues

Some functions, like handleIncomingRESDL and handleOutgoingRESDL, could potentially run out of gas if the amount of data being processed is large (e.g., if there are many locks). It's good practice to ensure that loops or extensive operations are kept within reasonable limits or can handle larger inputs efficiently.

3. Access Control

The setDelegatorPool function only checks if the caller is the owner. If ownership is transferred and the new owner is malicious, they could set an arbitrary delegator pool address. Consider adding a more robust access control mechanism to ensure that the delegator pool is only set to a safe address.

4. Missing Input Validation

In the migrate function, there is no validation on the _lockingDuration. If an excessively large value is passed in, it may cause issues with state variables. Add checks to ensure _lockingDuration is within acceptable bounds.

5. Event Emissions

While the contract does emit events for important actions, ensure that all significant state changes are covered by events. For example, if the delegatorPool is set, it would be helpful to emit an event for that action as well.

6. Unchecked Arithmetic Operations

While Solidity 0.8.x includes built-in overflow/underflow protection, it is still advisable to explicitly handle cases where underflows or overflows may occur. This includes checks before decrementing or incrementing values.

7. Potential for Front Running

Certain functions that update state and transfer tokens (like onTokenTransfer) could be susceptible to front-running attacks. You might consider implementing measures like committing and revealing to prevent such attacks.

8. Destructive State Changes Before Transfers

In functions like handleOutgoingRESDL, the lock is deleted from state before the token transfer. This could cause inconsistencies if the transfer fails for any reason. It might be better to ensure that the transfer succeeds before modifying the state.

9. Upgradeability Considerations

The contract uses the OpenZeppelin upgradeable pattern. Make sure that the logic for upgrades is secure, particularly regarding initializer functions and state management between versions.

10. State Manipulation via External Calls

In handleIncomingRESDL, external input is accepted without comprehensive validation (other than checking the lock owner). This can be risky if the external source is compromised. Ensure that all inputs are validated and consider adding checks for other relevant state variables.

Conclusion

Always conduct thorough testing, including unit tests and integration tests, especially for smart contracts that involve complex state management. A formal audit by a reputable firm is also recommended before deploying to a production environment.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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