🛡️ Validation Issues in Params Struct for Limits and Default Values

[ccamel]

Note

Severity: Low
target: v7.1.0 - Commit: 3c854270b006db30aa3894da2cdba10cc31b8c5f
Ref: OKP4 Blockchain Audit Report v1.0 - 02-05-2024 - BlockApex

Description

The Params struct in the okp4d (now axoned) blockchain encapsulates crucial operational settings within the logic module, including configurations for Interpreter, Limits, and GasPolicy. The Limits struct is especially critical as it controls thresholds for computational power (MaxGas), script storage capacity (MaxSize), query result limits (MaxResultCount), and user output size (MaxUserOutputSize). These parameters are intended to ensure the blockchain operates within safe and efficient computational
and storage boundaries.

However, a significant issue has arisen due to inadequate validation mechanisms. This issue exposes the network to risks of misconfiguration that can lead to DoS attacks, performance degradation, or network paralysis when exploited during network initialization or reconfiguration phases.

The validateLimits function is crucial for ensuring parameters stay within safe boundaries, but it's
currently unimplemented, marked only as a "TODO." This lack of validation allows potentially risky configurations that can be too restrictive or excessively lenient. Simultaneously, there are no set upper limits for key parameters like MaxGas, MaxSize, and MaxResultCount. Without these caps, values can be set extremely high, potentially leading to severe system strain from processing more results than manageable, causing significant performance issues or failures. These gaps in validation and
enforcement pose critical risks to system stability and efficiency.

Recommandation

Implement rigorous checks within the validateLimits function to verify that all parameters, particularly MaxGas, MaxSize, and MaxResultCount, fall within specified safe operational ranges.

[bdeneux]

MaxGas is no longer available (#706). However, for MaxSize, MaxUserOutputSize, MaxResultCount, and MaxVariables, accept nil or zero values to indicate no limit. I don't see any additional verification for these parameters to enforce maximum limits.

According to the documentation in the proto file, some parameters use nil or zero values to indicate unlimited (max_variables, max_user_output_size), while others use only nil to indicate unlimited (max_size, max_result_count). We should consider aligning the behavior for all parameters and updating the code implementation accordingly. If we decide that only nil should indicate unlimited, we should add a check to ensure that zero is not authorized. In other hand choose nil and zero value for indicate unlimited, I don't see any additional verification on it.
Predicate coast also use zero and nil value to indicate default value (1) (weighting_factor, default_predicate_cost).

Additionally, we could add verification for the predicate whitelist and blacklist filters to ensure that the given predicates exist. And the same for PredicateCost params.

@ccamel @amimart

[amimart]

Closing it as done by #713