Liquid Staking

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

contracts/linkStaking/OperatorVault.sol

Reviewing the provided Solidity code for the OperatorVault contract, several potential vulnerabilities and issues can be identified. Below are the notable ones:

1. Integer Underflow/Overflow

Although you are using SafeCast from OpenZeppelin, which protects against overflows and underflows, make sure all arithmetic operations properly handle edge cases. Since Solidity 0.8.x has built-in overflow checks, the additional use of SafeCast is often unnecessary unless you are converting types.

2. Reentrancy Risk

Functions that transfer tokens should be protected against reentrancy attacks. Although this contract does not appear to call external contracts directly before transferring tokens, it’s prudent to use a reentrancy guard (e.g., nonReentrant modifier from OpenZeppelin) in functions like withdraw, exitVault, and _withdrawRewards.

3. Lack of Access Control on Critical Functions

While there are checks in place to ensure that only the operator or the rewards receiver can execute certain functions, consider adding checks for critical functions like setOperator and setRewardsReceiver to ensure they can only be called by the owner of the contract or a designated admin role.

4. Gas Limit Issues

The raiseAlert function calculates the rewards based on the difference in token balances. If a malicious actor can manipulate the balance of the contract, this could lead to unexpected outcomes. Ensure that any external calls (like pfAlertsController.raiseAlert(_feed)) are safe and not subject to gas limit issues.

5. Potential Token Transfer Issues

When transferring tokens, it's a good practice to check the return value. In the safeTransfer and safeTransferFrom calls, ensure that these calls do not fail silently. While using OpenZeppelin's SafeERC20, it should revert on failure, but always verify this behavior.

6. Unclaimed Rewards Logic

The logic around calculating and managing unclaimedRewards might lead to inconsistencies if not managed carefully. Ensure that any change to unclaimedRewards accurately reflects the state of rewards.

7. Event Emission on State Changes

While events like AlertRaised, WithdrawRewards, and SetRewardsReceiver are emitted on state changes, ensure that any critical state changes or state-dependent logic are logged appropriately for auditing and debugging.

8. Operator Removal Logic

The function isRemoved checks if the operator has been removed from the Chainlink staking contract. Ensure that this check is reliable and cannot be manipulated by a malicious actor.

9. Handling Zero Addresses

There are several places in your code that check for zero addresses. While this is good practice, ensure that checks are consistently applied across the contract to prevent unintended behaviors.

10. Constructor Disablement

The _disableInitializers() method in the constructor disables further initialization, which is usually used in upgradeable contracts. Ensure this is indeed your desired behavior and consider how this affects contract upgrades.

11. Visibility of Functions

The visibility of functions like _withdrawRewards is set to private. Ensure that this is the intended access level and that no functions need to be public or external.

Recommendations:

  • Consider implementing reentrancy guards on critical functions.

  • Use event emissions for all significant state changes for better transparency.

  • Ensure robust error handling and logging.

  • Regularly conduct audits and security assessments to keep the contract safe against emerging vulnerabilities.

Overall, while the contract looks relatively well-structured, addressing the points mentioned above can significantly enhance its security and reliability.

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.