Fix Coverity (#22094)
* fix_coverity: Fix coverity issues * fix_coverity: Address cubic * fix_coverity: Address Copilot (P1) * fix_coverity: Address Copilot (P2) * fix_coverity: Address Copilot (P3) * fix_coverity: Address Copilot (P4)
thiagoftsm committed
Mar 31, 2026 at 19:26 UTC
5dd4257202e984f21c9b0c61e4a088041b27011b
4 files changed
+150
-15
src/ml/ml-unittest.cc
+107
@@ -757,6 +757,111 @@ static void test_parameter_combinations()
757
}
758
}
759
760
+// Test: timestamps > INT32_MAX must survive serialize -> deserialize unchanged.
761
+// Before the bounds-check fix, the (time_t) cast of json_object_get_int64()
762
+// would silently truncate on 32-bit time_t, breaking model ordering/pruning.
763
+static void test_kmeans_timestamp_roundtrip()
764
+{
765
+ fprintf(stderr, " test_kmeans_timestamp_roundtrip...\n");
766
+
767
+ // 3 000 000 000 > INT32_MAX (2 147 483 647). On 32-bit time_t the value
768
+ // doesn't fit, so skip — the guard would correctly reject it on the way in.
769
+ if (sizeof(time_t) < 8) {
770
+ fprintf(stderr, " skipped (time_t is 32-bit on this platform)\n");
771
+ return;
772
+ }
773
+
774
+ const time_t large_after = (time_t) 3000000000LL;
775
+ const time_t large_before = (time_t) 3000003600LL;
776
+
777
+ ml_kmeans_inlined_t original;
778
+ original.cluster_centers[0].set_size(6);
779
+ original.cluster_centers[1].set_size(6);
780
+ for (int i = 0; i < 6; i++) {
781
+ original.cluster_centers[0](i) = (double)(i + 1);
782
+ original.cluster_centers[1](i) = (double)(i + 7);
783
+ }
784
+ original.min_dist = 1.5;
785
+ original.max_dist = 9.5;
786
+ original.after = large_after;
787
+ original.before = large_before;
788
+
789
+ BUFFER *wb = buffer_create(0, NULL);
790
+ buffer_json_initialize(wb, "\"", "\"", 0, true, BUFFER_JSON_OPTIONS_MINIFY);
791
+ ml_kmeans_serialize(&original, wb);
792
+ buffer_json_finalize(wb);
793
+
794
+ struct json_object *root = json_tokener_parse(buffer_tostring(wb));
795
+ ML_TEST_ASSERT(root != NULL, "round-trip: serialized output must be valid JSON");
796
+
797
+ if (root) {
798
+ ml_kmeans_inlined_t result;
799
+ result.cluster_centers[0].set_size(6);
800
+ result.cluster_centers[1].set_size(6);
801
+
802
+ bool ok = ml_kmeans_deserialize(&result, root);
803
+ ML_TEST_ASSERT(ok, "round-trip: deserialize must succeed for large timestamp");
804
+
805
+ if (ok) {
806
+ ML_TEST_ASSERT(result.after == large_after,
807
+ "round-trip: 'after' must survive unchanged (> INT32_MAX)");
808
+ ML_TEST_ASSERT(result.before == large_before,
809
+ "round-trip: 'before' must survive unchanged (> INT32_MAX)");
810
+ }
811
+
812
+ json_object_put(root);
813
+ }
814
+
815
+ buffer_free(wb);
816
+}
817
+
818
+// Test: deserialize must reject models carrying negative timestamps.
819
+// Negative Unix timestamps are never valid for ML model windows.
820
+static void test_kmeans_timestamp_rejection()
821
+{
822
+ fprintf(stderr, " test_kmeans_timestamp_rejection...\n");
823
+
824
+ // Build a fully-valid kmeans JSON object and then override one timestamp
825
+ // field to an invalid value, verifying that ml_kmeans_deserialize rejects it.
826
+ auto make_full_root = [](int64_t after_val, int64_t before_val) -> struct json_object * {
827
+ struct json_object *r = json_object_new_object();
828
+ json_object_object_add(r, "after", json_object_new_int64(after_val));
829
+ json_object_object_add(r, "before", json_object_new_int64(before_val));
830
+ json_object_object_add(r, "min_dist", json_object_new_double(1.0));
831
+ json_object_object_add(r, "max_dist", json_object_new_double(9.0));
832
+
833
+ struct json_object *cc = json_object_new_array();
834
+ for (int c = 0; c < 2; c++) {
835
+ struct json_object *cv = json_object_new_array();
836
+ for (int i = 0; i < 6; i++)
837
+ json_object_array_add(cv, json_object_new_double((double)(c * 6 + i + 1)));
838
+ json_object_array_add(cc, cv);
839
+ }
840
+ json_object_object_add(r, "cluster_centers", cc);
841
+ return r;
842
+ };
843
+
844
+ {
845
+ struct json_object *r = make_full_root(-1LL, 100LL);
846
+ ml_kmeans_inlined_t km;
847
+ km.cluster_centers[0].set_size(6);
848
+ km.cluster_centers[1].set_size(6);
849
+ bool ok = ml_kmeans_deserialize(&km, r);
850
+ ML_TEST_ASSERT(!ok, "negative 'after' must be rejected");
851
+ json_object_put(r);
852
+ }
853
+
854
+ {
855
+ struct json_object *r = make_full_root(100LL, -1LL);
856
+ ml_kmeans_inlined_t km;
857
+ km.cluster_centers[0].set_size(6);
858
+ km.cluster_centers[1].set_size(6);
859
+ bool ok = ml_kmeans_deserialize(&km, r);
860
+ ML_TEST_ASSERT(!ok, "negative 'before' must be rejected");
861
+ json_object_put(r);
862
+ }
863
+}
864
+
865
extern "C" int ml_unittest()
866
{
867
fprintf(stderr, "\nML unit tests:\n");
@@ -781,6 +886,8 @@ extern "C" int ml_unittest()
886
test_preprocess_predict_equivalence();
887
test_constant_input();
888
test_parameter_combinations();
889
+ test_kmeans_timestamp_roundtrip();
890
+ test_kmeans_timestamp_rejection();
891
892
fprintf(stderr, "\nML tests: %d run, %d failed\n", tests_run, tests_failed);
893
src/ml/ml.cc
+21
-7
@@ -222,11 +222,11 @@ ml_dimension_add_model(const nd_uuid_t *metric_uuid, const ml_kmeans_inlined_t *
222
if (unlikely(rc != SQLITE_OK))
223
goto bind_fail;
224
225
- rc = sqlite3_bind_int(res, ++param, (int) inlined_km->after);
225
+ rc = sqlite3_bind_int64(res, ++param, (sqlite3_int64) inlined_km->after);
226
if (unlikely(rc != SQLITE_OK))
227
goto bind_fail;
228
229
- rc = sqlite3_bind_int(res, ++param, (int) inlined_km->before);
229
+ rc = sqlite3_bind_int64(res, ++param, (sqlite3_int64) inlined_km->before);
230
if (unlikely(rc != SQLITE_OK))
231
goto bind_fail;
232
@@ -297,7 +297,7 @@ ml_dimension_delete_models(const nd_uuid_t *metric_uuid, time_t before)
297
if (unlikely(rc != SQLITE_OK))
298
goto bind_fail;
299
300
- rc = sqlite3_bind_int(res, ++param, (int) before);
300
+ rc = sqlite3_bind_int64(res, ++param, (sqlite3_int64) before);
301
if (unlikely(rc != SQLITE_OK))
302
goto bind_fail;
303
@@ -344,9 +344,9 @@ ml_prune_old_models(size_t num_models_to_prune)
344
}
345
}
346
347
- int after = (int) (now_realtime_sec() - Cfg.delete_models_older_than);
347
+ time_t after = now_realtime_sec() - (time_t) Cfg.delete_models_older_than;
348
349
- rc = sqlite3_bind_int(res, ++param, after);
349
+ rc = sqlite3_bind_int64(res, ++param, (sqlite3_int64) after);
350
if (unlikely(rc != SQLITE_OK))
351
goto bind_fail;
352
@@ -429,8 +429,22 @@ int ml_dimension_load_models(RRDDIM *rd, sqlite3_stmt **active_stmt) {
429
while ((rc = sqlite3_step_monitored(res)) == SQLITE_ROW) {
430
ml_kmeans_t km;
431
432
- km.after = sqlite3_column_int(res, 0);
433
- km.before = sqlite3_column_int(res, 1);
432
+ sqlite3_int64 raw_after = sqlite3_column_int64(res, 0);
433
+ sqlite3_int64 raw_before = sqlite3_column_int64(res, 1);
434
+
435
+ // Protect against silent truncation when time_t is narrower than int64_t
436
+ // (e.g. 32-bit builds, corrupted DB, or far-future timestamps).
437
+ static constexpr sqlite3_int64 kTimeMin = std::numeric_limits<time_t>::min();
438
+ static constexpr sqlite3_int64 kTimeMax = std::numeric_limits<time_t>::max();
439
+ if (raw_after < kTimeMin || raw_after > kTimeMax ||
440
+ raw_before < kTimeMin || raw_before > kTimeMax) {
441
+ error_report("Skipping ML model row with out-of-range timestamps: after=%" PRId64 " before=%" PRId64,
442
+ (int64_t) raw_after, (int64_t) raw_before);
443
+ continue;
444
+ }
445
+
446
+ km.after = (time_t) raw_after;
447
+ km.before = (time_t) raw_before;
448
449
km.min_dist = sqlite3_column_double(res, 2);
450
km.max_dist = sqlite3_column_double(res, 3);
src/ml/ml_kmeans.cc
+16
-4
@@ -24,8 +24,8 @@ ml_kmeans_init(ml_kmeans_t *kmeans)
24
void
25
ml_kmeans_train(ml_kmeans_t *kmeans, const std::vector<DSample> &preprocessed_features, unsigned max_iters, time_t after, time_t before)
26
{
27
- kmeans->after = (uint32_t) after;
28
- kmeans->before = (uint32_t) before;
27
+ kmeans->after = after;
28
+ kmeans->before = before;
29
30
kmeans->min_dist = std::numeric_limits<calculated_number_t>::max();
31
kmeans->max_dist = std::numeric_limits<calculated_number_t>::min();
@@ -186,7 +186,13 @@ bool ml_kmeans_deserialize(ml_kmeans_inlined_t *inlined_km, struct json_object *
186
netdata_log_error("Failed to deserialize kmeans: failed to parse int for 'after'");
187
return false;
188
}
189
- inlined_km->after = json_object_get_int(value);
189
+ int64_t raw_after = json_object_get_int64(value);
190
+ // Timestamps must be non-negative Unix epoch seconds and fit in time_t.
191
+ if (raw_after < 0 || raw_after > (int64_t) std::numeric_limits<time_t>::max()) {
192
+ netdata_log_error("Failed to deserialize kmeans: out-of-range value for 'after': %" PRId64, raw_after);
193
+ return false;
194
+ }
195
+ inlined_km->after = (time_t) raw_after;
196
197
if (!json_object_object_get_ex(root, "before", &value)) {
198
netdata_log_error("Failed to deserialize kmeans: missing key 'before'");
@@ -196,7 +202,13 @@ bool ml_kmeans_deserialize(ml_kmeans_inlined_t *inlined_km, struct json_object *
202
netdata_log_error("Failed to deserialize kmeans: failed to parse int for 'before'");
203
return false;
204
}
199
- inlined_km->before = json_object_get_int(value);
205
+ int64_t raw_before = json_object_get_int64(value);
206
+ // Same contract as 'after': non-negative and fits in time_t.
207
+ if (raw_before < 0 || raw_before > (int64_t) std::numeric_limits<time_t>::max()) {
208
+ netdata_log_error("Failed to deserialize kmeans: out-of-range value for 'before': %" PRId64, raw_before);
209
+ return false;
210
+ }
211
+ inlined_km->before = (time_t) raw_before;
212
213
if (!json_object_object_get_ex(root, "min_dist", &value)) {
214
netdata_log_error("Failed to deserialize kmeans: missing key 'min_dist'");
src/ml/ml_kmeans.h
+6
-4
@@ -5,6 +5,8 @@
5
6
#include "ml_features.h"
7
8
+#include <ctime>
9
+
10
typedef struct web_buffer BUFFER;
11
12
struct ml_kmeans_inlined_t;
@@ -13,8 +15,8 @@ struct ml_kmeans_t {
15
std::vector<DSample> cluster_centers;
16
calculated_number_t min_dist;
17
calculated_number_t max_dist;
16
- uint32_t after;
17
- uint32_t before;
18
+ time_t after;
19
+ time_t before;
20
21
ml_kmeans_t() : min_dist(0), max_dist(0), after(0), before(0)
22
{
@@ -28,8 +30,8 @@ struct ml_kmeans_inlined_t {
30
std::array<DSample, 2> cluster_centers;
31
calculated_number_t min_dist;
32
calculated_number_t max_dist;
31
- uint32_t after;
32
- uint32_t before;
33
+ time_t after;
34
+ time_t before;
35
36
ml_kmeans_inlined_t() : min_dist(0), max_dist(0), after(0), before(0)
37
{