Liquid Staking

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

contracts/core/base/RewardsPoolController.sol

Here’s an analysis of the potential vulnerabilities or weaknesses in the provided smart contract:

1. Misconfiguration of UUPSUpgradeable (Upgradeability Risk)

  • Issue: The contract uses UUPSUpgradeable, which introduces upgradeability, but if _authorizeUpgrade() is improperly protected, unauthorized upgrades can occur. The current code restricts upgrades to onlyOwner, but it’s crucial to ensure ownership cannot be hijacked.

  • Potential Fix: Double-check that the ownership logic is secure and implement measures like OpenZeppelin’s AccessControl for more granular access control.


2. Lack of Input Validation

  • Issue:

    • In addToken(), the provided token and rewards pool addresses are directly accepted. If a malicious or incorrect address is added, it could disrupt rewards distribution or drain tokens.

    • In removeToken(), there is no check to prevent re-adding an already removed token later with different semantics or malicious pools.

  • Potential Fix:

    • Add address validation to ensure _token and _rewardsPool are valid ERC20 and rewards pool contracts.

    • Consider checking for duplicate tokens or whether the pool already exists.


3. Reentrancy Risk in distributeToken()

  • Issue: In distributeToken(), the contract transfers tokens and immediately calls an external function (distributeRewards()) on the reward pool. This sequence is vulnerable to reentrancy attacks if the distributeRewards() function has unintended side effects.

  • Potential Fix:

    • Implement the Checks-Effects-Interactions pattern by updating internal state before the token transfer and external call.

    • Add a reentrancy guard using OpenZeppelin’s ReentrancyGuard.


4. Gas Optimization Opportunity in Loops

  • Issue: The code contains multiple unbounded for loops, such as in distributeTokens(), withdrawRewards(), _updateRewards(), and tokenBalances(). If tokens becomes large, these loops could run out of gas, making the functions unusable.

  • Potential Fix:

    • Limit the length of tokens.

    • Use batch processing or allow partial operations to avoid running out of gas.


5. No Emergency Withdraw Mechanism

  • Issue: If the rewards pools become misconfigured or unusable, the contract holds ERC20 tokens but provides no emergency mechanism for owners or users to recover their funds.

  • Potential Fix:

    • Implement an emergency withdrawal function that allows the owner to recover stuck tokens or funds.


6. Unsafe Removal Logic in removeToken()

  • Issue: In removeToken(), when removing a token, the code shifts the last token into the current index and pops the array without cleaning up the associated state. This could lead to inconsistencies if rewards or balances are calculated based on indices.

  • Potential Fix:

    • Ensure the token pool mapping is always in sync with the token list. Alternatively, return an error or handle edge cases explicitly.


7. onTokenTransfer() ERC677 Compatibility Risk

  • Issue: Not all tokens follow the ERC677 standard. If a non-ERC677 token calls this function, it might behave unexpectedly or fail silently.

  • Potential Fix:

    • Add fallback mechanisms for non-ERC677 tokens to ensure that tokens not compatible with onTokenTransfer() can still be distributed properly.


8. Lack of Pausable Modifier for Critical Functions

  • Issue: Key functions like distributeTokens(), withdrawRewards(), and addToken() are not protected against misuse or attacks.

  • Potential Fix:

    • Add a pausable mechanism so that the owner can pause critical functionality in case of an emergency.


9. Token Pool Management Risk

  • Issue: If a rewards pool changes state or becomes malicious after being added, the contract lacks mechanisms to audit or revoke those pools automatically. This creates a risk of trust assumptions on third-party pools.

  • Potential Fix:

    • Add logic to ensure that only whitelisted pools can be added or maintain on-chain pool audits.


10. No Restriction on External Calls in withdrawRewards()

  • Issue: The function withdrawRewards() allows anyone to withdraw rewards from multiple tokens without any check on whether the token supports this. It could lead to unexpected failures if any token pool behaves abnormally.

  • Potential Fix:

    • Include a check to confirm that each token supports reward withdrawal before executing the function.


Summary of Fix Recommendations:

  1. Access Control: Secure upgradeability with robust ownership logic.

  2. Reentrancy Protection: Use ReentrancyGuard in external token interactions.

  3. Input Validation: Validate token and rewards pool addresses before adding.

  4. Gas Optimizations: Use batch processing or bounded loops.

  5. Emergency Functions: Add fallback withdrawal mechanisms.

  6. Consistency Checks: Ensure tokens array and mappings stay synchronized.

  7. Pausable: Add a pause mechanism to critical functions.

Updates

Lead Judging Commences

inallhonesty Lead Judge 8 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.