Add slot bounds check and unit tests for pluginsd_parser (#22598)
* 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.
Stelios Fragkakis committed
Jun 8, 2026 at 12:03 UTC
883928b21d077bee9e8ee46d1066c3e7677652c4
5 files changed
+109
-12
src/database/rrdset-pluginsd-array.h
+6
@@ -48,7 +48,13 @@ typedef struct pluginsd_rrddim_array {
48
// --------------------------------------------------------------------------------------------------------------------
49
50
// Create a new array with the specified size and refcount=1
51
+// size is expected to be bounded by the caller: pluginsd slot input is capped at
52
+// the parser (PLUGINSD_DIMENSION_SLOT_MAX) and the no-slots path uses the dimension
53
+// count, so the size multiplication below cannot overflow. The check documents and
54
+// guards that invariant against any future unbounded caller (debug builds only).
55
static inline PRD_ARRAY *prd_array_create(size_t size) {
56
+ internal_fatal(size > (SIZE_MAX - sizeof(PRD_ARRAY)) / sizeof(struct pluginsd_rrddim),
57
+ "PRD_ARRAY: requested size %zu would overflow the allocation", size);
58
PRD_ARRAY *arr = callocz(1, sizeof(PRD_ARRAY) + size * sizeof(struct pluginsd_rrddim));
59
arr->refcount = 1;
60
arr->size = size;
src/plugins.d/pluginsd_internals.h
+26
-3
@@ -369,13 +369,36 @@ static inline RRDSET *pluginsd_find_chart(RRDHOST *host, const char *chart, cons
369
return st;
370
}
371
372
-static ALWAYS_INLINE ssize_t pluginsd_parse_rrd_slot(char **words, size_t num_words) {
372
+static ALWAYS_INLINE ssize_t pluginsd_parse_rrd_slot(char **words, size_t num_words, size_t max_slot) {
373
+ // Contract: max_slot must fit in ssize_t so the (ssize_t) cast below stays
374
+ // non-negative. All callers pass small compile-time caps (PLUGINSD_*_SLOT_MAX),
375
+ // so for the inlined constant this check is folded away by the compiler.
376
+ internal_fatal(max_slot > (size_t)SSIZE_MAX,
377
+ "PLUGINSD: max_slot %zu exceeds SSIZE_MAX", max_slot);
378
+
379
ssize_t slot = -1;
380
char *id = get_word(words, num_words, 1);
381
+ // Words are NUL-terminated and && short-circuits left-to-right, so each id[k]
382
+ // is read only after id[0..k-1] matched the non-NUL "SLOT" chars. A shorter word
383
+ // (e.g. "X" or "SLOT") fails an earlier comparison or hits the terminator at id[4],
384
+ // so these fixed-index reads never go past the token's NUL. No bounds check needed.
385
if(id && id[0] == PLUGINSD_KEYWORD_SLOT[0] && id[1] == PLUGINSD_KEYWORD_SLOT[1] &&
386
id[2] == PLUGINSD_KEYWORD_SLOT[2] && id[3] == PLUGINSD_KEYWORD_SLOT[3] && id[4] == ':') {
377
- slot = (ssize_t) str2ull_encoded(&id[5]);
378
- if(slot < 0) slot = 0; // to make the caller increment its idx of the words
387
+ unsigned long long parsed_slot = str2ull_encoded(&id[5]);
388
+ if(unlikely(parsed_slot > max_slot)) {
389
+ nd_log_limit_static_global_var(erl_slot, 1, 0);
390
+ nd_log_limit(&erl_slot, NDLS_COLLECTORS, NDLP_WARNING,
391
+ "PLUGINSD: ignoring invalid SLOT value '%s' above the supported maximum %zu",
392
+ &id[5], max_slot);
393
+ // slot 0 means: the SLOT word was present (so the caller still advances its
394
+ // word index), but the value is unusable as a cache index. Each caller
395
+ // handles slot < 1 per its own path -- chart lookups fall back to finding
396
+ // the chart by id, and dimension caching switches to the non-slotted
397
+ // (by-position) path (pluginsd_rrddim_put_to_slot sets dims_with_slots=false).
398
+ slot = 0;
399
+ }
400
+ else
401
+ slot = (ssize_t)parsed_slot;
402
}
403
404
return slot;
src/plugins.d/pluginsd_parser.c
+71
-6
@@ -6,7 +6,7 @@
6
7
static inline PARSER_RC pluginsd_set(char **words, size_t num_words, PARSER *parser) {
8
int idx = 1;
9
- ssize_t slot = pluginsd_parse_rrd_slot(words, num_words);
9
+ ssize_t slot = pluginsd_parse_rrd_slot(words, num_words, PLUGINSD_DIMENSION_SLOT_MAX);
10
if(slot >= 0) idx++;
11
12
char *dimension = get_word(words, num_words, idx++);
@@ -39,7 +39,7 @@ static inline PARSER_RC pluginsd_set(char **words, size_t num_words, PARSER *par
39
40
static inline PARSER_RC pluginsd_begin(char **words, size_t num_words, PARSER *parser) {
41
int idx = 1;
42
- ssize_t slot = pluginsd_parse_rrd_slot(words, num_words);
42
+ ssize_t slot = pluginsd_parse_rrd_slot(words, num_words, PLUGINSD_CHART_SLOT_MAX);
43
if(slot >= 0) idx++;
44
45
char *id = get_word(words, num_words, idx++);
@@ -356,7 +356,7 @@ static inline PARSER_RC pluginsd_chart(char **words, size_t num_words, PARSER *p
356
if(!host) return PLUGINSD_DISABLE_PLUGIN(parser, NULL, NULL);
357
358
int idx = 1;
359
- ssize_t slot = pluginsd_parse_rrd_slot(words, num_words);
359
+ ssize_t slot = pluginsd_parse_rrd_slot(words, num_words, PLUGINSD_CHART_SLOT_MAX);
360
if(slot >= 0) idx++;
361
362
char *type = get_word(words, num_words, idx++);
@@ -473,7 +473,7 @@ static inline PARSER_RC pluginsd_chart(char **words, size_t num_words, PARSER *p
473
474
static inline PARSER_RC pluginsd_dimension(char **words, size_t num_words, PARSER *parser) {
475
int idx = 1;
476
- ssize_t slot = pluginsd_parse_rrd_slot(words, num_words);
476
+ ssize_t slot = pluginsd_parse_rrd_slot(words, num_words, PLUGINSD_DIMENSION_SLOT_MAX);
477
if(slot >= 0) idx++;
478
479
char *id = get_word(words, num_words, idx++);
@@ -804,7 +804,7 @@ static ALWAYS_INLINE PARSER_RC pluginsd_begin_v2(char **words, size_t num_words,
804
timing_init();
805
806
int idx = 1;
807
- ssize_t slot = pluginsd_parse_rrd_slot(words, num_words);
807
+ ssize_t slot = pluginsd_parse_rrd_slot(words, num_words, PLUGINSD_CHART_SLOT_MAX);
808
if(slot >= 0) idx++;
809
810
char *id = get_word(words, num_words, idx++);
@@ -949,7 +949,7 @@ static ALWAYS_INLINE PARSER_RC pluginsd_set_v2(char **words, size_t num_words, P
949
timing_init();
950
951
int idx = 1;
952
- ssize_t slot = pluginsd_parse_rrd_slot(words, num_words);
952
+ ssize_t slot = pluginsd_parse_rrd_slot(words, num_words, PLUGINSD_DIMENSION_SLOT_MAX);
953
if(slot >= 0) idx++;
954
955
char *dimension = get_word(words, num_words, idx++);
@@ -1513,7 +1513,72 @@ void parser_init_repertoire(PARSER *parser, PARSER_REPERTOIRE repertoire) {
1513
}
1514
}
1515
1516
+static int pluginsd_parser_unittest_slot_bounds(size_t max_slot) {
1517
+ // The boundary cases below build "max_slot - 1", so a zero cap would underflow.
1518
+ // All real callers pass nonzero compile-time caps; guard against misuse anyway.
1519
+ if(max_slot < 1) {
1520
+ netdata_log_error("PLUGINSD: slot bounds unittest requires max_slot >= 1, got %zu", max_slot);
1521
+ return 1;
1522
+ }
1523
+
1524
+ // Note on initialization: every element below is given an explicit
1525
+ // initializer, so C zero-fills the remainder of each slot_word array. The
1526
+ // trailing three entries start empty and are filled from max_slot at runtime.
1527
+ struct slot_test_case {
1528
+ char slot_word[64];
1529
+ ssize_t expected;
1530
+ } cases[] = {
1531
+ { "", -1 }, // no SLOT word -> -1 (caller must not advance idx)
1532
+ { PLUGINSD_KEYWORD_SLOT ":0", 0 }, // explicit zero -> uncached
1533
+ { PLUGINSD_KEYWORD_SLOT ":1", 1 }, // smallest cached slot
1534
+ { PLUGINSD_KEYWORD_SLOT ":-1", 0 }, // negative parses as unsigned 0 -> uncached
1535
+ { PLUGINSD_KEYWORD_SLOT ":abc", 0 }, // malformed decimal -> 0 -> uncached
1536
+ { PLUGINSD_KEYWORD_SLOT ":0xZZ", 0 }, // malformed hex -> 0 -> uncached
1537
+ { PLUGINSD_KEYWORD_SLOT ":0x0AAAAAAAAAAAAAAB", 0 }, // over cap; cast stays positive, would wrap allocation
1538
+ { PLUGINSD_KEYWORD_SLOT ":0xFFFFFFFFFFFFFFFF", 0 }, // u64 max -> over cap -> uncached
1539
+ { PLUGINSD_KEYWORD_SLOT ":0x40000000", 0 }, // over both caps -> uncached (the reported OOM value)
1540
+ { "", 0 }, // filled below: max_slot - 1 (accepted)
1541
+ { "", 0 }, // filled below: max_slot (accepted, boundary)
1542
+ { "", 0 }, // filled below: max_slot + 1 (rejected, boundary)
1543
+ };
1544
+
1545
+ const size_t n = _countof(cases);
1546
+
1547
+ snprintfz(cases[n - 3].slot_word, sizeof(cases[n - 3].slot_word),
1548
+ PLUGINSD_KEYWORD_SLOT ":%zu", max_slot - 1);
1549
+ cases[n - 3].expected = (ssize_t)(max_slot - 1);
1550
+
1551
+ snprintfz(cases[n - 2].slot_word, sizeof(cases[n - 2].slot_word),
1552
+ PLUGINSD_KEYWORD_SLOT ":%zu", max_slot);
1553
+ cases[n - 2].expected = (ssize_t)max_slot;
1554
+
1555
+ snprintfz(cases[n - 1].slot_word, sizeof(cases[n - 1].slot_word),
1556
+ PLUGINSD_KEYWORD_SLOT ":%zu", max_slot + 1);
1557
+ cases[n - 1].expected = 0;
1558
+
1559
+ for(size_t i = 0; i < _countof(cases); i++) {
1560
+ char command[] = "DIMENSION";
1561
+ char *words[] = { command, cases[i].slot_word[0] ? cases[i].slot_word : NULL };
1562
+ size_t num_words = words[1] ? 2 : 1;
1563
+
1564
+ ssize_t slot = pluginsd_parse_rrd_slot(words, num_words, max_slot);
1565
+ if(slot != cases[i].expected) {
1566
+ netdata_log_error("PLUGINSD: slot parser unittest failed for '%s': expected %zd, got %zd",
1567
+ words[1] ? words[1] : "(unset)", cases[i].expected, slot);
1568
+ return 1;
1569
+ }
1570
+ }
1571
+
1572
+ return 0;
1573
+}
1574
+
1575
int pluginsd_parser_unittest(void) {
1576
+ if(pluginsd_parser_unittest_slot_bounds(PLUGINSD_DIMENSION_SLOT_MAX))
1577
+ return 1;
1578
+
1579
+ if(pluginsd_parser_unittest_slot_bounds(PLUGINSD_CHART_SLOT_MAX))
1580
+ return 1;
1581
+
1582
PARSER *p = parser_init(NULL, -1, -1, PARSER_INPUT_SPLIT, NULL);
1583
pluginsd_keywords_init(p, PARSER_INIT_PLUGINSD | PARSER_INIT_STREAMING);
1584
src/plugins.d/pluginsd_parser.h
+3
@@ -18,6 +18,9 @@
18
#define PLUGINSD_MAX_DEFERRED_SIZE (100 * 1024 * 1024)
19
20
#define PLUGINSD_MIN_RRDSET_POINTERS_CACHE 1024
21
+// Slots are cache indexes. Larger values are treated as uncached input to avoid sparse cache allocations.
22
+#define PLUGINSD_CHART_SLOT_MAX 1000000
23
+#define PLUGINSD_DIMENSION_SLOT_MAX 65535
24
25
// PARSER return codes
26
typedef enum __attribute__ ((__packed__)) parser_rc {
src/plugins.d/pluginsd_replication.c
+3
-3
@@ -111,7 +111,7 @@ PARSER_RC pluginsd_chart_definition_end(char **words, size_t num_words, PARSER *
111
112
ALWAYS_INLINE PARSER_RC pluginsd_replay_begin(char **words, size_t num_words, PARSER *parser) {
113
int idx = 1;
114
- ssize_t slot = pluginsd_parse_rrd_slot(words, num_words);
114
+ ssize_t slot = pluginsd_parse_rrd_slot(words, num_words, PLUGINSD_CHART_SLOT_MAX);
115
if(slot >= 0) idx++;
116
117
char *id = get_word(words, num_words, idx++);
@@ -216,7 +216,7 @@ ALWAYS_INLINE PARSER_RC pluginsd_replay_begin(char **words, size_t num_words, PA
216
217
ALWAYS_INLINE PARSER_RC pluginsd_replay_set(char **words, size_t num_words, PARSER *parser) {
218
int idx = 1;
219
- ssize_t slot = pluginsd_parse_rrd_slot(words, num_words);
219
+ ssize_t slot = pluginsd_parse_rrd_slot(words, num_words, PLUGINSD_DIMENSION_SLOT_MAX);
220
if(slot >= 0) idx++;
221
222
char *dimension = get_word(words, num_words, idx++);
@@ -284,7 +284,7 @@ ALWAYS_INLINE PARSER_RC pluginsd_replay_rrddim_collection_state(char **words, si
284
return PARSER_RC_OK;
285
286
int idx = 1;
287
- ssize_t slot = pluginsd_parse_rrd_slot(words, num_words);
287
+ ssize_t slot = pluginsd_parse_rrd_slot(words, num_words, PLUGINSD_DIMENSION_SLOT_MAX);
288
if(slot >= 0) idx++;
289
290
char *dimension = get_word(words, num_words, idx++);