Liquid Staking

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

test/linkStaking/operator-staking-pool.test.ts

To identify vulnerabilities and propose improvements for the given code, we’ll look at the following areas:

  1. Reentrancy Protection

  2. Access Control

  3. Gas Limit Issues

  4. Error Handling

  5. State Validation

  6. Event Emission

Here’s a detailed examination of the code along with proposed improvements:

1. Reentrancy Protection

Vulnerability:
The withdraw and transferAndCall functions could be vulnerable to reentrancy attacks if they involve any external calls that can invoke the same contract again.

Improvement:

  • Use a mutex pattern or the nonReentrant modifier from OpenZeppelin's ReentrancyGuard for functions that make external calls.

2. Access Control

Vulnerability:
Operators can be added or removed without any limitations based on the role or authority of the caller. If there are no checks in place, this could allow unauthorized accounts to manipulate the operator list.

Improvement:

  • Implement role-based access control using the Ownable or AccessControl contracts from OpenZeppelin. Ensure only authorized accounts can call addOperators and removeOperators.

3. Gas Limit Issues

Vulnerability:
The addOperators and removeOperators functions can take multiple addresses. If a large number of addresses are provided, it could lead to high gas costs or out-of-gas errors.

Improvement:

  • Set a reasonable limit on the number of addresses that can be added or removed in a single transaction. For example, limit to 10 addresses per call.

4. Error Handling

Vulnerability:
Custom errors like InvalidToken, SenderNotAuthorized, etc., must be carefully handled and tested. Incorrect error handling might lead to silent failures.

Improvement:

  • Ensure comprehensive unit tests are in place for all error conditions. Use require statements for checks within your smart contracts that ensure valid states before proceeding.

5. State Validation

Vulnerability:
The contract does not appear to validate whether the lst token is correctly set up or whether it has the required allowance for transfers.

Improvement:

  • Check for token approval in the onTokenTransfer method and ensure that the token is properly initialized. Use require statements to enforce that.

6. Event Emission

Vulnerability:
There are no event emissions after state-changing actions like addOperators, removeOperators, and withdraw. This can lead to difficulties in tracking actions and debugging.

Improvement:

  • Emit events after every significant state change to improve transparency and allow for better tracking of contract activities. For instance:

    event OperatorAdded(address indexed operator);
    event OperatorRemoved(address indexed operator);
    event Withdrawn(address indexed operator, uint256 amount);

Suggested Code Improvements

Here’s an example of how some of the changes could be implemented in the OperatorStakingPool contract:

import "@openzeppelin/contracts/access/AccessControl.sol";
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
contract OperatorStakingPool is AccessControl, ReentrancyGuard {
bytes32 public constant OPERATOR_ROLE = keccak256("OPERATOR_ROLE");
constructor(address lstToken, uint256 depositLimit) {
// Setup roles
_setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
_setupRole(OPERATOR_ROLE, msg.sender);
// Initialize other state variables...
}
modifier onlyOperator() {
require(hasRole(OPERATOR_ROLE, msg.sender), "SenderNotAuthorized()");
_;
}
function addOperators(address[] memory operators) external onlyRole(DEFAULT_ADMIN_ROLE) {
require(operators.length <= 10, "Too many operators");
for (uint256 i = 0; i < operators.length; i++) {
require(!hasRole(OPERATOR_ROLE, operators[i]), "OperatorAlreadyAdded()");
grantRole(OPERATOR_ROLE, operators[i]);
emit OperatorAdded(operators[i]);
}
}
function removeOperators(address[] memory operators) external onlyRole(DEFAULT_ADMIN_ROLE) {
for (uint256 i = 0; i < operators.length; i++) {
require(hasRole(OPERATOR_ROLE, operators[i]), "OperatorNotFound()");
revokeRole(OPERATOR_ROLE, operators[i]);
emit OperatorRemoved(operators[i]);
}
}
function withdraw(uint256 amount) external onlyOperator nonReentrant {
// Ensure amount is valid...
// Perform the withdrawal
emit Withdrawn(msg.sender, amount);
}
// ...other functions...
}

Conclusion

The improvements proposed above aim to strengthen the security and robustness of the OperatorStakingPool contract. It’s crucial to thoroughly test all changes in various scenarios to ensure that the contract functions as intended without vulnerabilities. Additionally, incorporating good coding practices like error handling and event emissions will enhance the overall quality and maintainability of the code.

Updates

Lead Judging Commences

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