Liquid Staking

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

test/core/sdlPool/linear-boost-controller.test.ts

Below is a security and vulnerability analysis of the code, focusing on potential issues:


1. Potential Integer Overflow or Underflow Risks

  • Context: Solidity (depending on the version) does not have automatic checks for overflows/underflows unless SafeMath or Solidity 0.8+ is used, which includes built-in overflow protection.

  • Risk: If the underlying contract (i.e., LinearBoostController) performs calculations with large numbers (e.g., 5 * 2 * 365 * DAY), there could be overflow issues if the calculations aren’t protected.

Mitigation:

  • Ensure that the contract uses Solidity 0.8+ or SafeMath to prevent overflows/underflows in boost calculations.


2. Insufficient Validation on Input Values

  • Observation: The test ensures that the function getBoostAmount() reverts with a custom error when provided with invalid durations. This is a good practice. However:

    • Edge Case: What happens with negative or extremely large inputs? For example, durations like -1, 2^256 - 1, or edge inputs might cause unexpected behavior.

Mitigation:

  • Ensure input validation in the smart contract (LinearBoostController) to handle edge cases like negative or exceptionally large inputs, even if they aren’t allowed by the user interface.


3. State Modifiability without Access Control Checks

  • Issue: The test shows that the maxBoost and maxLockingDuration can be modified using setMaxBoost() and setMaxLockingDuration(). If these functions are public and not restricted (e.g., missing onlyOwner or equivalent access control modifiers), unauthorized entities might manipulate these values.

Mitigation:

  • Confirm that setMaxBoost() and setMaxLockingDuration() in the contract are protected with proper access control (e.g., onlyOwner or Ownable).


4. Accuracy of Floating-Point Comparisons

  • Observation: The test converts results to fixed-point numbers using .toFixed(5) and then compares them with expected values. Floating-point arithmetic is inherently imprecise.

Risk: Minor precision differences between Solidity and JavaScript number handling could lead to incorrect test outcomes (e.g., due to rounding).

Mitigation:

  • Instead of directly comparing numbers, use tolerance-based comparisons, like:

    const actual = Number(fromEther(await boostController.getBoostAmount(toEther(5), DAY)));
    assert.closeTo(actual, 0.0137, 0.00001); // Allow a small margin of error

5. Tests for Reverted Errors Could Be More Granular

  • Observation: The test checks if getBoostAmount() reverts with InvalidLockingDuration() for certain invalid inputs. This is good, but other error scenarios may also arise, such as:

    • Inputs exactly at boundary conditions (like 0 or maxLockingDuration).

    • Testing with unexpected inputs to detect potential reentrancy issues or internal arithmetic bugs.

Mitigation:

  • Consider adding more test cases to cover edge inputs and unexpected scenarios.


6. Missing Test Coverage for Events or State Changes

  • Observation: The tests do not verify whether calling setMaxBoost() or setMaxLockingDuration() emits events or changes internal state as expected.

Mitigation:

  • Add assertions to confirm that the appropriate events are emitted, if any, and that the internal state matches the expected values after updates.


7. Use of loadFixture() and Reusability

  • Observation: loadFixture() helps speed up tests by caching deployments, which is a good practice. However, ensure the fixture is correctly reset to avoid unintended state leakage between tests, especially if future tests involve more complex state updates.


Summary of Potential Vulnerabilities and Recommendations:

  1. Overflow/Underflow Risk: Ensure Solidity 0.8+ or SafeMath usage.

  2. Access Control: Ensure setMaxBoost() and setMaxLockingDuration() are protected.

  3. Edge Case Inputs: Add more edge tests (negative, large values, boundary inputs).

  4. Floating-Point Precision: Use assert.closeTo() for number comparisons.

  5. Comprehensive Test Coverage: Test state changes and events after updates.

  6. Fixture Usage: Ensure proper state isolation between tests.

If the underlying contract addresses these issues, this test suite provides a solid foundation for verifying the correctness of boost calculations.

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.