Liquid Staking

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

contracts/linkStaking/test/StakingRewardsMock.sol

The provided Solidity code for the StakingRewardsMock contract has several potential vulnerabilities and areas for improvement that should be considered:

  1. No Access Control on setReward:

    • The setReward function allows anyone to set the reward for any staker. This can lead to manipulation of rewards by malicious actors. To mitigate this, access control should be implemented, for example, using OpenZeppelin’s Ownable or AccessControl.

    import "@openzeppelin/contracts/access/Ownable.sol"; // or AccessControl
    contract StakingRewardsMock is Ownable {
    ...
    function setReward(address _staker, uint256 _amount) external onlyOwner {
    rewards[_staker] = _amount;
    }
    }
  2. Potential for Reentrancy Attack:

    • While safeTransfer from OpenZeppelin's SafeERC20 is used, which is generally safe, it's important to ensure that the contract does not become susceptible to reentrancy attacks, especially if the token being transferred is also a contract. The common practice is to use the Checks-Effects-Interactions pattern to prevent reentrancy issues.

    • You should set rewards[msg.sender] = 0; before the token transfer:

    function claimReward() external {
    uint256 reward = rewards[msg.sender];
    require(reward != 0, "No reward to claim");
    rewards[msg.sender] = 0; // Update state before transfer
    token.safeTransfer(msg.sender, reward);
    }
  3. Lack of Event Emission:

    • The claimReward and setReward functions do not emit events. Emitting events for significant state changes (like claiming rewards or setting rewards) helps with tracking activity on the blockchain and is useful for off-chain applications.

    event RewardClaimed(address indexed staker, uint256 amount);
    event RewardSet(address indexed staker, uint256 amount);
    function claimReward() external {
    ...
    emit RewardClaimed(msg.sender, reward);
    }
    function setReward(address _staker, uint256 _amount) external onlyOwner {
    rewards[_staker] = _amount;
    emit RewardSet(_staker, _amount);
    }
  4. Gas Limit and Token Transfer Issues:

    • While safeTransfer handles token transfer errors gracefully, if the token contract has an issue (like being a non-compliant ERC20), this could lead to silent failures. It’s important to ensure the token follows the expected standards.

  5. Incorrect Token Interface:

    • Ensure that the IERC677 interface is implemented correctly by the token being used. The IERC677 interface typically extends the standard ERC20, and ensure that it contains all necessary methods and events.

  6. No Check for Address Zero:

    • The constructor should check that the _token address passed is not the zero address to prevent issues in future interactions with the token.

    constructor(address _token) {
    require(_token != address(0), "Token address cannot be zero");
    token = IERC677(_token);
    }
  7. Potential Overflow/Underflow:

    • In Solidity version 0.8.0 and above, arithmetic operations automatically revert on overflow or underflow. However, in a prior version, you would have to use SafeMath. Just ensure that you're aware of this when considering upgrades or maintaining backward compatibility.

By addressing these concerns, the contract can be made more secure and robust.

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.