Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[M-02] Insufficient Timestamp Validation in Dilution Events #52

Open
softstackio opened this issue Sep 9, 2024 · 0 comments
Open

[M-02] Insufficient Timestamp Validation in Dilution Events #52

softstackio opened this issue Sep 9, 2024 · 0 comments

Comments

@softstackio
Copy link

Description:
The ClaimContract contains two significant issues related to the validation of dilution event timestamps:

  1. Lack of Minimum Time Gap: While the contract ensures that each dilution timestamp is greater than the previous one, it does not enforce a minimum time gap between dilution events. This could allow dilution events to occur in rapid succession, potentially disrupting the intended gradual dilution process.
  2. Missing Upper Bound Check: The contract does not implement an upper bound check for dilution timestamps. This oversight allows for the setting of extremely large timestamp values, which could lead to overflow issues or render dilution events practically impossible to occur.

Impact:
These issues could have several negative consequences:

  1. Rapid Dilution: Without a minimum time gap, the contract owner could set dilution
    timestamps very close to each other, potentially diluting user balances more quickly than
    intended. This could undermine user trust and the fairness of the claiming process.
  2. Permanent Token Lock: Setting extremely high timestamp values could effectively lock
    tokens indefinitely, as the dilution events would never occur within a reasonable
    timeframe.
  3. Potential Overflow: Large timestamp values might cause overflow issues in other parts
    of the contract or in external systems interacting with it.

Proof of Concept:
Consider the following scenario:

/ Current block.timestamp: 1000000000 (May 2001)
ClaimContract contract = new ClaimContract(
   beneficorAddress1,
   beneficorAddress2,
   "prefix",
   1000000001, // 1 second after deployment
   1000000002, // 2 seconds after deployment
   1000000003  // 3 seconds after deployment
);

In this example, all dilution events could occur within 3 seconds, which is likely not the intended behavior for a gradual dilution process.
Alternatively:

// Current block.timestamp: 1000000000 (May 2001)
ClaimContract contract = new ClaimContract(
   beneficorAddress1,
   beneficorAddress2,
   "prefix",
11579208923731619542357098500868790785326998466564056403945758400791312963
9935, // max uint256 value
11579208923731619542357098500868790785326998466564056403945758400791312963
9935,
11579208923731619542357098500868790785326998466564056403945758400791312963
9935
);

This setup would effectively prevent any dilution from ever occurring, as the timestamps are set to the maximum possible value for uint256.
Recommendation:
To address these issues, consider implementing the following changes:

  1. Enforce Minimum Time Gaps:
    Add checks in the constructor to ensure a minimum time difference between dilution events:
uint256 constant MIN_DILUTION_GAP = 30 days;
constructor(...) {
   // Existing checks
   if (_dilute_s2_50_timestamp <= _dilute_s1_75_timestamp +
MIN_DILUTION_GAP)
       revert InsufficientDilutionGap();
   if (_dilute_s3_0_timestamp <= _dilute_s2_50_timestamp +
MIN_DILUTION_GAP)
       revert InsufficientDilutionGap();
// ...
}
  1. Add a reasonable upper limit for dilution timestamps:
uint256 constant MAX_TIMESTAMP = 2524608000; // January 1, 2050
constructor(...) {
   // Existing checks
   if (_dilute_s1_75_timestamp > MAX_TIMESTAMP) revert TimestampTooHigh();
   if (_dilute_s2_50_timestamp > MAX_TIMESTAMP) revert TimestampTooHigh();
   if (_dilute_s3_0_timestamp > MAX_TIMESTAMP) revert TimestampTooHigh();
   // ...
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant