@cryptotaxi247 / netdata-1 / commits / f2d263716

Fix heap-use-after-free in query progress updates (#20431)

The issue occurred when web_client was freed while plugin threads were still trying to access it through progress callbacks. The progress callback stored a pointer to web_client, which became invalid after the connection was closed. Solution: Modified the progress callback signature to include the transaction UUID as a parameter, eliminating the need to store web_client pointers. Changes: - Updated rrd_function_progress_cb_t typedef to include nd_uuid_t *transaction - Modified all progress callback implementations to use the transaction parameter - Updated all callback invocations to pass the transaction from inflight structures - Removed web_client pointer dependencies by passing NULL as callback data This ensures that progress updates only use data that remains valid for the lifetime of the function execution.

Costa Tsaousis committed Jun 6, 2025 at 17:29 UTC f2d26371623ebebbcd93b5e3e7742b0fdc07976b

8 files changed +16 -12

src/daemon/dyncfg/dyncfg-intercept.c

+2 -2

@@ -239,7 +239,7 @@ static void dyncfg_apply_action_on_all_template_jobs(struct rrd_function_execute

239 dfe_done(df);

240

241 if(rfe->progress.cb)

242 - rfe->progress.cb(rfe->progress.data, done, all);

242 + rfe->progress.cb(rfe->transaction, rfe->progress.data, done, all);

243

244 dfe_start_reentrant(dyncfg_globals.nodes, df) {

245 if(df->template == template && df->type == DYNCFG_TYPE_JOB) {

@@ -253,7 +253,7 @@ static void dyncfg_apply_action_on_all_template_jobs(struct rrd_function_execute

253 dyncfg_echo(df_dfe.item, df, df_dfe.name, cmd_to_send_to_plugin);

254

255 if(rfe->progress.cb)

256 - rfe->progress.cb(rfe->progress.data, ++done, all);

256 + rfe->progress.cb(rfe->transaction, rfe->progress.data, ++done, all);

257 }

258 }

259 dfe_done(df);

src/database/rrdfunctions.h

+1 -1

@@ -16,7 +16,7 @@ typedef void (*rrd_function_result_callback_t)(BUFFER *wb, int code, void *resul

16 typedef bool (*rrd_function_is_cancelled_cb_t)(void *is_cancelled_cb_data);

17 typedef void (*rrd_function_cancel_cb_t)(void *data);

18 typedef void (*rrd_function_register_canceller_cb_t)(void *register_cancel_cb_data, rrd_function_cancel_cb_t cancel_cb, void *cancel_cb_data);

19 -typedef void (*rrd_function_progress_cb_t)(void *data, size_t done, size_t all);

19 +typedef void (*rrd_function_progress_cb_t)(nd_uuid_t *transaction, void *data, size_t done, size_t all);

20 typedef void (*rrd_function_progresser_cb_t)(const char *transaction, void *data);

21 typedef void (*rrd_function_register_progresser_cb_t)(void *register_progresser_cb_data, rrd_function_progresser_cb_t progresser_cb, void *progresser_cb_data);

src/plugins.d/pluginsd_functions.c

+1 -1

@@ -437,7 +437,7 @@ PARSER_RC pluginsd_function_progress(char **words, size_t num_words, PARSER *par

437 size_t all = all_str && *all_str ? str2u(all_str) : 0;

438

439 if(pf->progress.cb)

440 - pf->progress.cb(pf->progress.data, done, all);

440 + pf->progress.cb(&pf->transaction, pf->progress.data, done, all);

441 }

442

443 return PARSER_RC_OK;

src/streaming/stream-sender-execute.c

+5 -2

@@ -39,15 +39,18 @@ static void stream_execute_function_callback(BUFFER *func_wb, int code, void *da

39 freez(tmp);

40 }

42 -static void stream_execute_function_progress_callback(void *data, size_t done, size_t all) {

42 +static void stream_execute_function_progress_callback(nd_uuid_t *transaction, void *data, size_t done, size_t all) {

43 struct inflight_stream_function *tmp = data;

44 struct sender_state *s = tmp->sender;

46 if(rrdhost_can_stream_metadata_to_parent(s->host)) {

47 CLEAN_BUFFER *wb = buffer_create(0, NULL);

49 + char transaction_str[UUID_COMPACT_STR_LEN];

50 + uuid_unparse_lower_compact(*transaction, transaction_str);

51 +

52 buffer_sprintf(wb, PLUGINSD_KEYWORD_FUNCTION_PROGRESS " '%s' %zu %zu\n",

50 - string2str(tmp->transaction), done, all);

53 + transaction_str, done, all);

55 sender_commit_clean_buffer(s, wb, STREAM_TRAFFIC_TYPE_FUNCTIONS);

56 }

src/web/api/v1/api_v1_config.c

+1 -1

@@ -84,7 +84,7 @@ int api_v1_config(RRDHOST *host, struct web_client *w, char *url __maybe_unused)

84 int code = rrd_function_run(host, w->response.data, timeout, w->user_auth.access, cmd,

85 true, transaction,

86 NULL, NULL,

87 - web_client_progress_functions_update, w,

87 + web_client_progress_functions_update, NULL,

88 web_client_interrupt_callback, w,

89 w->payload, buffer_tostring(source), false);

src/web/api/v1/api_v1_function.c

+1 -1

@@ -38,7 +38,7 @@ int api_v1_function(RRDHOST *host, struct web_client *w, char *url) {

39 return rrd_function_run(host, wb, timeout, w->user_auth.access, function, true, transaction,

40 NULL, NULL,

41 - web_client_progress_functions_update, w,

41 + web_client_progress_functions_update, NULL,

42 web_client_interrupt_callback, w, w->payload,

43 buffer_tostring(source), false);

44 }

src/web/api/web_api.c

+4 -3

@@ -144,9 +144,10 @@ void nd_web_api_init(void) {

144 time_grouping_init();

145 }

146

147 -void web_client_progress_functions_update(void *data, size_t done, size_t all) {

147 +void web_client_progress_functions_update(nd_uuid_t *transaction, void *data, size_t done, size_t all) {

148 // handle progress updates from the plugin

149 - struct web_client *w = data;

150 - query_progress_functions_update(&w->transaction, done, all);

149 + // data parameter is no longer used - transaction is provided directly

150 + (void)data;

151 + query_progress_functions_update(transaction, done, all);

152 }

153

src/web/api/web_api.h

+1 -1

@@ -20,7 +20,7 @@ struct web_client;

21 void nd_web_api_init(void);

23 -void web_client_progress_functions_update(void *data, size_t done, size_t all);

23 +void web_client_progress_functions_update(nd_uuid_t *transaction, void *data, size_t done, size_t all);

25 void host_labels2json(RRDHOST *host, BUFFER *wb, const char *key);