more strict parsing of the output of system-info.sh (#19745)
* more strict parsing of the output of system-info.sh * empty values are warnings * fix anonymous analytics too * fix anonymous analytics again
Costa Tsaousis committed
Mar 2, 2025 at 16:09 UTC
9f9d2649888a3ebe7a6c4dfd2d9cdc22886660a5
2 files changed
+135
-89
src/daemon/analytics.c
+48
-57
@@ -874,74 +874,67 @@ bool analytics_check_enabled(void) {
874
}
875
876
void analytics_statistic_send(const analytics_statistic_t *statistic) {
877
- if (!statistic || !statistic->action || !analytics_check_enabled() || !analytics_script_exists())
877
+ if (!statistic || !statistic->action || !*statistic->action|| !analytics_check_enabled() || !analytics_script_exists())
878
return;
879
880
const char *action_result = statistic->result;
881
const char *action_data = statistic->data;
882
883
- if (!statistic->result)
884
- action_result = "";
885
-
886
- if (!statistic->data)
887
- action_data = "";
888
-
889
- char *command_to_run = mallocz(
890
- sizeof(char) * (strlen(statistic->action) + strlen(action_result) + strlen(action_data) + FILENAME_MAX +
891
- analytics_data.data_length + (ANALYTICS_NO_OF_ITEMS * 3) + 15));
892
- sprintf(
893
- command_to_run,
883
+ CLEAN_BUFFER *cmd = buffer_create(0, NULL);
884
+ buffer_sprintf(
885
+ cmd,
886
"%s/anonymous-statistics.sh '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' ",
887
netdata_configured_primary_plugins_dir,
888
statistic->action,
897
- action_result,
898
- action_data,
899
- analytics_data.netdata_config_stream_enabled,
900
- analytics_data.netdata_config_memory_mode,
901
- analytics_data.netdata_config_exporting_enabled,
902
- analytics_data.netdata_exporting_connectors,
903
- analytics_data.netdata_allmetrics_prometheus_used,
904
- analytics_data.netdata_allmetrics_shell_used,
905
- analytics_data.netdata_allmetrics_json_used,
906
- analytics_data.netdata_dashboard_used,
907
- analytics_data.netdata_collectors,
908
- analytics_data.netdata_collectors_count,
909
- analytics_data.netdata_buildinfo,
910
- analytics_data.netdata_config_page_cache_size,
911
- analytics_data.netdata_config_multidb_disk_quota,
912
- analytics_data.netdata_config_https_enabled,
913
- analytics_data.netdata_config_web_enabled,
914
- analytics_data.netdata_config_release_channel,
915
- analytics_data.netdata_mirrored_host_count,
916
- analytics_data.netdata_mirrored_hosts_reachable,
917
- analytics_data.netdata_mirrored_hosts_unreachable,
918
- analytics_data.netdata_notification_methods,
919
- analytics_data.netdata_alarms_normal,
920
- analytics_data.netdata_alarms_warning,
921
- analytics_data.netdata_alarms_critical,
922
- analytics_data.netdata_charts_count,
923
- analytics_data.netdata_metrics_count,
924
- analytics_data.netdata_config_is_parent,
925
- analytics_data.netdata_config_hosts_available,
926
- analytics_data.netdata_host_cloud_available,
927
- analytics_data.netdata_host_aclk_available,
928
- analytics_data.netdata_host_aclk_protocol,
929
- analytics_data.netdata_host_aclk_implementation,
930
- analytics_data.netdata_host_agent_claimed,
931
- analytics_data.netdata_host_cloud_enabled,
932
- analytics_data.netdata_config_https_available,
933
- analytics_data.netdata_install_type,
934
- analytics_data.netdata_config_is_private_registry,
935
- analytics_data.netdata_config_use_private_registry,
936
- analytics_data.netdata_config_oom_score,
937
- analytics_data.netdata_prebuilt_distro,
938
- analytics_data.netdata_fail_reason);
889
+ action_result ? action_result : "",
890
+ action_data ? action_data : "",
891
+ analytics_data.netdata_config_stream_enabled ? analytics_data.netdata_config_stream_enabled : "",
892
+ analytics_data.netdata_config_memory_mode ? analytics_data.netdata_config_memory_mode : "",
893
+ analytics_data.netdata_config_exporting_enabled ? analytics_data.netdata_config_exporting_enabled : "",
894
+ analytics_data.netdata_exporting_connectors ? analytics_data.netdata_exporting_connectors : "",
895
+ analytics_data.netdata_allmetrics_prometheus_used ? analytics_data.netdata_allmetrics_prometheus_used : "",
896
+ analytics_data.netdata_allmetrics_shell_used ? analytics_data.netdata_allmetrics_shell_used : "",
897
+ analytics_data.netdata_allmetrics_json_used ? analytics_data.netdata_allmetrics_json_used : "",
898
+ analytics_data.netdata_dashboard_used ? analytics_data.netdata_dashboard_used : "",
899
+ analytics_data.netdata_collectors ? analytics_data.netdata_collectors : "",
900
+ analytics_data.netdata_collectors_count ? analytics_data.netdata_collectors_count : "",
901
+ analytics_data.netdata_buildinfo ? analytics_data.netdata_buildinfo : "",
902
+ analytics_data.netdata_config_page_cache_size ? analytics_data.netdata_config_page_cache_size : "",
903
+ analytics_data.netdata_config_multidb_disk_quota ? analytics_data.netdata_config_multidb_disk_quota : "",
904
+ analytics_data.netdata_config_https_enabled ? analytics_data.netdata_config_https_enabled : "",
905
+ analytics_data.netdata_config_web_enabled ? analytics_data.netdata_config_web_enabled : "",
906
+ analytics_data.netdata_config_release_channel ? analytics_data.netdata_config_release_channel : "",
907
+ analytics_data.netdata_mirrored_host_count ? analytics_data.netdata_mirrored_host_count : "",
908
+ analytics_data.netdata_mirrored_hosts_reachable ? analytics_data.netdata_mirrored_hosts_reachable : "",
909
+ analytics_data.netdata_mirrored_hosts_unreachable ? analytics_data.netdata_mirrored_hosts_unreachable : "",
910
+ analytics_data.netdata_notification_methods ? analytics_data.netdata_notification_methods : "",
911
+ analytics_data.netdata_alarms_normal ? analytics_data.netdata_alarms_normal : "",
912
+ analytics_data.netdata_alarms_warning ? analytics_data.netdata_alarms_warning : "",
913
+ analytics_data.netdata_alarms_critical ? analytics_data.netdata_alarms_critical : "",
914
+ analytics_data.netdata_charts_count ? analytics_data.netdata_charts_count : "",
915
+ analytics_data.netdata_metrics_count ? analytics_data.netdata_metrics_count : "",
916
+ analytics_data.netdata_config_is_parent ? analytics_data.netdata_config_is_parent : "",
917
+ analytics_data.netdata_config_hosts_available ? analytics_data.netdata_config_hosts_available : "",
918
+ analytics_data.netdata_host_cloud_available ? analytics_data.netdata_host_cloud_available : "",
919
+ analytics_data.netdata_host_aclk_available ? analytics_data.netdata_host_aclk_available : "",
920
+ analytics_data.netdata_host_aclk_protocol ? analytics_data.netdata_host_aclk_protocol : "",
921
+ analytics_data.netdata_host_aclk_implementation ? analytics_data.netdata_host_aclk_implementation : "",
922
+ analytics_data.netdata_host_agent_claimed ? analytics_data.netdata_host_agent_claimed : "",
923
+ analytics_data.netdata_host_cloud_enabled ? analytics_data.netdata_host_cloud_enabled : "",
924
+ analytics_data.netdata_config_https_available ? analytics_data.netdata_config_https_available : "",
925
+ analytics_data.netdata_install_type ? analytics_data.netdata_install_type : "",
926
+ analytics_data.netdata_config_is_private_registry ? analytics_data.netdata_config_is_private_registry : "",
927
+ analytics_data.netdata_config_use_private_registry ? analytics_data.netdata_config_use_private_registry : "",
928
+ analytics_data.netdata_config_oom_score ? analytics_data.netdata_config_oom_score : "",
929
+ analytics_data.netdata_prebuilt_distro ? analytics_data.netdata_prebuilt_distro : "",
930
+ analytics_data.netdata_fail_reason ? analytics_data.netdata_fail_reason : ""
931
+ );
932
933
nd_log(NDLS_DAEMON, NDLP_DEBUG,
934
"%s/anonymous-statistics.sh '%s' '%s' '%s'",
935
netdata_configured_primary_plugins_dir, statistic->action, action_result, action_data);
936
944
- POPEN_INSTANCE *instance = spawn_popen_run(command_to_run);
937
+ POPEN_INSTANCE *instance = spawn_popen_run(buffer_tostring(cmd));
938
if (instance) {
939
char buffer[4 + 1];
940
char *s = fgets(buffer, 4, spawn_popen_stdout(instance));
@@ -962,8 +955,6 @@ void analytics_statistic_send(const analytics_statistic_t *statistic) {
955
nd_log(NDLS_DAEMON, NDLP_NOTICE,
956
"Failed to run statistics script: %s/anonymous-statistics.sh",
957
netdata_configured_primary_plugins_dir);
965
-
966
- freez(command_to_run);
958
}
959
960
void analytics_reset(void) {
src/database/rrdhost-system-info.c
+87
-32
@@ -239,46 +239,101 @@ void rrdhost_system_info_to_rrdlabels(struct rrdhost_system_info *system_info, R
239
240
int rrdhost_system_info_detect(struct rrdhost_system_info *system_info) {
241
#if !defined(OS_WINDOWS)
242
- char *script;
243
- script = mallocz(sizeof(char) * (strlen(netdata_configured_primary_plugins_dir) + strlen("system-info.sh") + 2));
244
- sprintf(script, "%s/%s", netdata_configured_primary_plugins_dir, "system-info.sh");
245
- if (unlikely(access(script, R_OK) != 0)) {
246
- netdata_log_error("System info script %s not found.",script);
247
- freez(script);
242
+ if (unlikely(!system_info)) {
243
+ netdata_log_error("SYSTEM INFO: System info structure is NULL.");
244
return 1;
245
}
246
251
- POPEN_INSTANCE *instance = spawn_popen_run(script);
252
- if(instance) {
253
- char line[200 + 1];
254
- // Removed the double strlens, if the Coverity tainted string warning reappears I'll revert.
255
- // One time init code, but I'm curious about the warning...
256
- while (fgets(line, 200, spawn_popen_stdout(instance)) != NULL) {
257
- char *value=line;
258
- while (*value && *value != '=') value++;
259
- if (*value=='=') {
260
- *value='\0';
261
- value++;
262
- char *end = value;
263
- while (*end && *end != '\n') end++;
264
- *end = '\0'; // Overwrite newline if present
265
- coverity_remove_taint(line); // I/O is controlled result of system_info.sh - not tainted
266
- coverity_remove_taint(value);
267
-
268
- if(unlikely(rrdhost_system_info_set_by_name(system_info, line, value))) {
269
- netdata_log_error("Unexpected environment variable %s=%s", line, value);
270
- } else {
271
- nd_setenv(line, value, 1);
272
- }
273
- }
247
+ CLEAN_BUFFER *script = buffer_create(0, NULL);
248
+ buffer_sprintf(script, "%s/system-info.sh", netdata_configured_primary_plugins_dir);
249
+
250
+ POPEN_INSTANCE *instance = NULL;
251
+ int ret = 1;
252
+
253
+ // Check if script exists and is readable
254
+ if (unlikely(access(buffer_tostring(script), R_OK) != 0)) {
255
+ netdata_log_error("SYSTEM INFO: System info script %s not found or not readable.",
256
+ buffer_tostring(script));
257
+ goto cleanup;
258
+ }
259
+
260
+ // Run the script
261
+ instance = spawn_popen_run(buffer_tostring(script));
262
+ if (unlikely(!instance)) {
263
+ netdata_log_error("SYSTEM INFO: Failed to execute system info script %s.",
264
+ buffer_tostring(script));
265
+ goto cleanup;
266
+ }
267
+
268
+ char line[1024];
269
+ FILE *fp = spawn_popen_stdout(instance);
270
+ if (unlikely(!fp)) {
271
+ netdata_log_error("SYSTEM INFO: Failed to get stdout from system info script.");
272
+ goto cleanup;
273
+ }
274
+
275
+ // Process each line from the script output
276
+ while (fgets(line, sizeof(line) - 1, fp) != NULL) {
277
+ // Ensure null-termination
278
+ line[sizeof(line) - 1] = '\0';
279
+
280
+ // Find the equals sign separator
281
+ char *value = strchr(line, '=');
282
+ if (unlikely(!value)) {
283
+ // Skip lines without an equal sign
284
+ nd_log(NDLS_DAEMON, NDLP_ERR,
285
+ "SYSTEM INFO: Skipping malformed line from system-info.sh (no '=' found): '%s'",
286
+ line);
287
+ continue;
288
+ }
289
+
290
+ // Split the name and value
291
+ *value = '\0';
292
+ value++;
293
+
294
+ // Trim any trailing newline from the value
295
+ char *end = strchr(value, '\n');
296
+ if (end) *end = '\0';
297
+
298
+ // Remove any carriage return that might be present (especially for macOS)
299
+ end = strchr(value, '\r');
300
+ if (end) *end = '\0';
301
+
302
+ // Validate name and value
303
+ if (unlikely(!*line || !*value)) {
304
+ nd_log(NDLS_DAEMON, NDLP_WARNING,
305
+ "SYSTEM INFO: Skipping empty name or value from system-info.sh: '%s=%s'",
306
+ line, value);
307
+ continue;
308
+ }
309
+
310
+ // Process the name-value pair
311
+ coverity_remove_taint(line);
312
+ coverity_remove_taint(value);
313
+
314
+ if (unlikely(rrdhost_system_info_set_by_name(system_info, line, value))) {
315
+ nd_log(NDLS_DAEMON, NDLP_ERR,
316
+ "SYSTEM INFO: Unexpected variable '%s=%s'",
317
+ line, value);
318
+ } else {
319
+ // Only set as environment variable if it was successfully processed
320
+ nd_setenv(line, value, 1);
321
}
275
- spawn_popen_wait(instance);
322
}
277
- freez(script);
323
+
324
+ // Everything succeeded
325
+ ret = 0;
326
+
327
+cleanup:
328
+ // Clean up resources
329
+ if (instance)
330
+ spawn_popen_wait(instance);
331
+
332
+ return ret;
333
#else
334
netdata_windows_get_system_info(system_info);
280
-#endif
335
return 0;
336
+#endif
337
}
338
339
void rrdhost_system_info_free(struct rrdhost_system_info *system_info) {