Add slot bounds check and unit tests for pluginsd_parser by stelfrag · Pull Request #22598 · netdata/netdata

stelfrag · 2026-06-01T08:35:34Z

Summary

Fixes parent-agent crashes caused by bad SLOT: values in streaming/pluginsd data.
A misbehaving or malicious child/plugin could send a huge slot number, making the parent try to grab a massive chunk of memory (crash) or write past the end of an array (corruption).
This PR puts sensible upper limits on those values.
Oversized slots are now safely ignored (with a rate-limited warning) and the data still gets processed normally via the regular lookup path — nothing is dropped.
Separate, generous limits for charts (1,000,000) and dimensions (65,535), so real-world setups are never affected.
Added tests that confirm valid slots still work and out-of-range values are rejected.

Summary by cubic

Adds slot bounds checking in the pluginsd parser to prevent crashes from oversized SLOT values, plus a guard against array allocation overflows. Clarifies fallback behavior when slots are invalid so charts and dimensions keep working without data loss.

Bug Fixes
- Enforce per-command max slot caps (charts 1,000,000; dimensions 65,535) across begin/chart/dimension/set and replication paths.
- Validate max_slot in pluginsd_parse_rrd_slot (must fit ssize_t) and treat over-limit values as uncached with a rate-limited warning.
- Clarify slot < 1 handling: charts fall back to id lookup; dimensions use the non-slotted path.
- Add allocation overflow guard in prd_array_create.
Tests
- Add slot-bounds unit tests covering valid, boundary, malformed/negative/hex, and overflow cases for both chart and dimension limits.
- Add guard for max_slot >= 1 to prevent underflow in boundary tests and simplify snprintfz usage.

^{Written for commit 9f9a178. Summary will update on new commits.}

- Introduced `PLUGINSD_SLOT_MAX` to define maximum supported slot value. - Added bounds checking to ignore invalid slot values exceeding the defined maximum. - Implemented `pluginsd_parser_unittest_slot_bounds` to validate slot parsing logic. - Updated error logging for invalid slot scenarios to improve debugging clarity.

- Replaced single `PLUGINSD_SLOT_MAX` with `PLUGINSD_CHART_SLOT_MAX` and `PLUGINSD_DIMENSION_SLOT_MAX` for separate chart and dimension slot limits. - Updated `pluginsd_parse_rrd_slot` to accept a maximum slot parameter for flexible slot validation. - Added unit tests to validate parsing logic with new slot bounds. - Adjusted error logging for invalid slots to reflect the updated configurable limits.

cubic-dev-ai

No issues found across 4 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

Architecture diagram

sequenceDiagram
    participant Plugin as Plugin/Child
    participant Parser as pluginsd Parser
    participant Log as Log System
    participant Lookup as Normal Lookup Path
    participant Cache as Slot Cache

    Note over Plugin,Cache: SLOT validation flow for streaming/pluginsd data

    Plugin->>Parser: CLIENT (sends BEGIN/SET/CHART/DIMENSION command with SLOT: value)
    Parser->>Parser: Parse SLOT:xxx from command words
    
    alt Parsed SLOT <= max_slot
        Note over Parser: SLOT valid (0 to max_slot)
        Parser->>Parser: Use as cache index
        Parser->>Cache: Access slot N
        Cache-->>Parser: Cached data
        Parser->>Lookup: Process via slot path
    else Parsed SLOT > max_slot or invalid
        Note over Parser: SLOT out of range (e.g., huge or > 1,000,000 / 65,535)
        Parser->>Log: rate-limited warning: "ignoring invalid SLOT value"
        Parser->>Parser: Set slot = 0 (safe fallback)
        Parser->>Lookup: Process via normal (uncached) lookup path
        Lookup-->>Parser: Data handled correctly
    end

    Note over Parser,Lookup: Two distinct max_slot constants applied by command type:
    Parser->>Parser: CHART/SET/BEGIN commands → PLUGINSD_CHART_SLOT_MAX (1,000,000)
    Parser->>Parser: DIMENSION/SET commands → PLUGINSD_DIMENSION_SLOT_MAX (65,535)

    Note over Plugins,Lookup: Replication flow (separate path)
    Plugin->>Parser: Sends replay_begin / replay_set / replay_rrddim
    Parser->>Parser: Same slot validation applied
    alt Valid SLOT
        Parser->>Cache: Use slot index
    else Invalid SLOT
        Parser->>Log: Rate-limited warning
        Parser->>Lookup: Fallback to normal path
    end

_{Re-trigger cubic}

Copilot

Pull request overview

This PR hardens the pluginsd/streaming parser against malformed or malicious SLOT: values by adding per-command upper bounds and unit tests, preventing sparse cache allocations and potential crashes/corruption while still processing incoming data via the normal lookup path.

Changes:

Add PLUGINSD_CHART_SLOT_MAX (1,000,000) and PLUGINSD_DIMENSION_SLOT_MAX (65,535) and pass the appropriate bound to each SLOT:-aware command/parser path.
Update replication replay parsing to apply the same slot bounds.
Add unit tests covering in-range slots and out-of-range behavior for both chart and dimension limits.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
`src/plugins.d/pluginsd_internals.h`	Updates slot parsing helper to accept a max bound and reject oversized slot values with rate-limited warnings.
`src/plugins.d/pluginsd_parser.h`	Introduces chart/dimension slot maximum constants used by the parser.
`src/plugins.d/pluginsd_parser.c`	Applies per-command max slot limits and adds unit tests validating slot bound behavior.
`src/plugins.d/pluginsd_replication.c`	Applies the same bounded slot parsing to replication replay commands.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…arser - Extended `pluginsd_parser_unittest_slot_bounds` with additional test cases for invalid, boundary, and overflow slots. - Added allocation overflow guard in `prd_array_create` for safer memory handling. - Improved `pluginsd_parse_rrd_slot` logic with detailed comments to clarify slot parsing behavior and edge cases.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

…fety

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

…ittest_slot_bounds` - Added a guard to ensure `max_slot >= 1` to prevent underflow in boundary cases. - Removed unnecessary `- 1` from `snprintfz` size calculations for cleaner logic.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

- Updated behavior for `slot < 1` to specify unique downstream handling for charts and dimensions. - Improved comments to better explain fallback logic and slot path scenarios.

sonarqubecloud · 2026-06-01T11:12:39Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

* Add slot bounds check and unit tests for pluginsd_parser - Introduced `PLUGINSD_SLOT_MAX` to define maximum supported slot value. - Added bounds checking to ignore invalid slot values exceeding the defined maximum. - Implemented `pluginsd_parser_unittest_slot_bounds` to validate slot parsing logic. - Updated error logging for invalid slot scenarios to improve debugging clarity. * Increase slot limits and refactor slot handling in pluginsd_parser - Replaced single `PLUGINSD_SLOT_MAX` with `PLUGINSD_CHART_SLOT_MAX` and `PLUGINSD_DIMENSION_SLOT_MAX` for separate chart and dimension slot limits. - Updated `pluginsd_parse_rrd_slot` to accept a maximum slot parameter for flexible slot validation. - Added unit tests to validate parsing logic with new slot bounds. - Adjusted error logging for invalid slots to reflect the updated configurable limits. * Add slot bounds checks, unit tests, and overflow guard for pluginsd_parser - Extended `pluginsd_parser_unittest_slot_bounds` with additional test cases for invalid, boundary, and overflow slots. - Added allocation overflow guard in `prd_array_create` for safer memory handling. - Improved `pluginsd_parse_rrd_slot` logic with detailed comments to clarify slot parsing behavior and edge cases. * Add `max_slot` bounds check in `pluginsd_parse_rrd_slot` to ensure safety * Add bounds check and improve `snprintfz` usage in `pluginsd_parser_unittest_slot_bounds` - Added a guard to ensure `max_slot >= 1` to prevent underflow in boundary cases. - Removed unnecessary `- 1` from `snprintfz` size calculations for cleaner logic. * Clarify handling of invalid slots in `pluginsd_internals`: - Updated behavior for `slot < 1` to specify unique downstream handling for charts and dimensions. - Improved comments to better explain fallback logic and slot path scenarios. (cherry picked from commit 883928b)

stelfrag added 2 commits June 1, 2026 10:04

github-actions Bot added the area/plugins.d label Jun 1, 2026

cubic-dev-ai Bot reviewed Jun 1, 2026

View reviewed changes

stelfrag marked this pull request as ready for review June 1, 2026 09:23

Copilot AI review requested due to automatic review settings June 1, 2026 09:23

stelfrag requested a review from Ferroin as a code owner June 1, 2026 09:23

stelfrag marked this pull request as draft June 1, 2026 09:24

Copilot started reviewing on behalf of stelfrag June 1, 2026 09:24 View session