Liquid Staking

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

contracts/linkStaking/test/StakingMock.sol

The provided Solidity code contains several potential vulnerabilities and areas for improvement that could affect the security and functionality of the StakingMock contract. Here are some key concerns:

1. Reentrancy Vulnerability

The unstake function transfers tokens to the user after modifying the staker's state. This can lead to reentrancy attacks where an attacker calls the unstake function recursively during the token transfer, leading to unexpected states.

Mitigation: Use the Checks-Effects-Interactions pattern. Move the state updates before the token transfer.

2. Lack of Access Control

Functions like removeOperator, setActive, setMaxPoolSize, and setDepositLimits can be called by any address. This could lead to unauthorized modifications.

Mitigation: Implement access control using Ownable or a custom modifier to restrict access to these critical functions.

3. Underflow and Overflow Risk

Even though Solidity 0.8.x includes built-in overflow and underflow checks, it’s worth mentioning the following cases:

  • The slashOperator function does not check if the _amount exceeds the current principal. This could result in an underflow, as the operation could attempt to subtract a larger value than the current principal.

Mitigation: Add require checks to ensure _amount is less than or equal to stakers[_operator].principal.

4. Uncontrolled Variables

The parameters like depositMin, depositMax, and maxPoolSize can be adjusted without checks. If these values are set incorrectly, they could lock funds or break the staking logic.

Mitigation: Ensure that the new values are valid (e.g., depositMin should always be less than depositMax).

5. Incorrect Staker State Management

The state management within onTokenTransfer can lead to incorrect data states if not handled carefully. Specifically, resetting unbondingPeriodEndsAt and claimPeriodEndsAt without checks may lead to inconsistencies.

Mitigation: Ensure that you only delete these values if they are expected to be reset under the conditions of the staking logic.

6. Potential for Token Transfer Failures

The token transfer might fail, which would lead to state inconsistencies. The safeTransfer function does help prevent this, but ensure that the token being interacted with follows the expected behavior (i.e., that it conforms to IERC677).

7. Improper Handling of Zero Principal

The logic in unstake permits unstaking of 0 principal, which may not be meaningful or expected.

Mitigation: Ensure proper validation to prevent unintended behavior from zero values.

8. Gas Limit Issues

In the removeOperator function, removing a user might require multiple state updates. If a staker has a large amount of data, it could lead to hitting gas limits in a single transaction.

Mitigation: Consider structuring the removal process or batch processing for large states.

9. Lack of Events

There are no events emitted for significant actions (e.g., when a user unstakes, deposits, or is removed). This makes it difficult to track changes on-chain.

Mitigation: Add events for critical state changes to facilitate off-chain tracking.

Conclusion

Addressing the above vulnerabilities and considerations will enhance the security and reliability of the StakingMock contract. Always ensure comprehensive testing and consider performing a formal audit before deploying contracts to production environments.

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.