Liquid Staking

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

contracts/linkStaking/base/Vault.sol

The provided Solidity contract appears to implement a vault for depositing LINK tokens into a Chainlink staking controller. However, as with any smart contract, there are potential vulnerabilities and best practices to consider. Here are some observations:

1. Upgradeability Risks

  • Initialization and Upgrade Logic: The contract uses OpenZeppelin's upgradeability pattern. It's important to ensure that the initializer is called correctly and that the contract is only upgraded by authorized accounts (in this case, the owner). Any flaws in this logic can lead to unexpected behavior if an unauthorized entity can upgrade the contract.

2. ERC20 Token Transfer

  • Reentrancy Attacks: The deposit and withdraw functions transfer tokens and interact with external contracts. If the stakeController or vaultController has a fallback function that calls back into the vault, it could lead to reentrancy attacks. You should consider implementing a reentrancy guard (e.g., using OpenZeppelin’s ReentrancyGuard) to prevent this.

3. Access Control

  • Vault Controller: The onlyVaultController modifier restricts access to certain functions. Ensure that the vaultController is securely set and cannot be changed to an unauthorized address after initialization, or implement a mechanism to change it with proper checks.

4. Error Handling

  • Transfer Failures: The safeTransferFrom and safeTransfer functions will revert if the transfer fails. While this is generally safe, it is good practice to handle potential errors from external calls gracefully, especially if interacting with contracts you do not control.

5. Gas Limit and Fallback Functions

  • If any of the external contracts (stakeController, vaultController, etc.) do not handle gas limits correctly, it might lead to failed transactions when executing the deposit, withdraw, or other functions. Always ensure external contract functions are designed to handle the expected gas usage.

6. Use of address Type

  • The addresses for the various controllers should be validated (not zero addresses) before being assigned to ensure that they point to valid contracts. Consider adding checks in the initializer function to verify that the provided addresses are not zero.

7. Potential for Outdated Contract

  • Since the contract depends on external contracts (like IStaking and IStakingRewards), ensure that those contracts are secure and well-audited. Bugs in those contracts could affect your vault’s operations.

8. Gas Optimization

  • There are opportunities for gas optimization in certain functions. For example, in the getTotalDeposits function, it may be more efficient to cache the results of getPrincipalDeposits() and getRewards() rather than calling those functions separately if the same state is accessed multiple times.

9. Lack of Events

  • The contract lacks events for critical operations (like deposits, withdrawals, and unbonding). Emitting events is crucial for tracking contract interactions and can assist with debugging and analytics.

Recommendations:

  • Implement reentrancy guards on sensitive functions.

  • Add input validation for addresses in the initializer.

  • Emit events for all state-changing operations.

  • Consider caching external calls when possible to save gas and reduce complexity.

  • Ensure that external contracts you interact with are audited and secure.

By addressing these points, you can enhance the security and robustness of your Vault contract. Always consider a thorough audit from a reputable security firm for any production-level contracts.

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.