Fix memory-safety and correctness bugs surfaced by Coverity audit (part 2) (#22267)
* claim: terminate public key read buffer Coverity CID 501623 (CHECKED_RETURN): `send_curl_request()` read `public.pem` into a fixed buffer without reserving space for a trailing NUL, then passed it through the JSON string path as a C string. Read at most `sizeof(public_key) - 1` bytes, keep the byte count, and terminate the buffer explicitly before use. * claim: guard missing 422 errorMsgKey Coverity CID 501624 (FORWARD_NULL): guard the 422 errorMsgKey comparisons in send_curl_request() when the claim server omits or mis-types errorMsgKey. Fall back to the existing generic 422 failure message instead of dereferencing a NULL string. * claim: cap claim response body size The claim flow buffered the full HTTP response body with no upper bound, which allowed a hostile endpoint to force unbounded growth during the curl transfer. Cap the response at 10 MiB in the write callback and configure libcurl to reject oversized responses early when the size is advertised. * claim: handle curl option setup failures Coverity CID 501625 (CHECKED_RETURN): stop ignoring curl_easy_setopt() errors when preparing claim requests. Abort the request setup with a clear failure reason instead of continuing with partially applied curl options such as a rejected proxy configuration. * claim: check curl_slist_append return value curl_slist_append() can return NULL on allocation failure. Keep the original list pointer, assign the new list to a temporary, and only commit it once the append succeeded. Fail the request cleanly with can_retry=false if the header cannot be appended, so the claim request never proceeds without its Content-Type header. * claim: guard size * nmemb overflow in response write callback `size * nmemb` can theoretically overflow size_t. Treat overflow as a too-large response (same as the existing size-limit path): flag it on the response buffer and return 0 so libcurl aborts the transfer. * claim: remove shared uuid scratch from getter Coverity CID 410065 (MISSING_LOCK): claim_id_get_uuid() copied the shared claim UUID through a function-local static and returned it after dropping claim.spinlock. Use a stack-local ND_UUID so concurrent readers do not race on the helper's scratch storage. --------- Co-authored-by: Costa Tsaousis <costa@netdata.cloud>
Stelios Fragkakis committed
Apr 25, 2026 at 16:01 UTC
8b118b61edae8777192491148621cec89accd071
2 files changed
+112
-39
src/claim/claim-with-api.c
+111
-38
@@ -9,6 +9,13 @@
9
#include <openssl/pem.h>
10
#include <openssl/err.h>
11
12
+#define CLAIM_RESPONSE_SIZE_LIMIT (10 * 1024 * 1024)
13
+
14
+struct claim_response_buffer {
15
+ BUFFER *wb;
16
+ bool too_large;
17
+};
18
+
19
static bool check_and_generate_certificates() {
20
FILE *fp;
21
EVP_PKEY *pkey = NULL;
@@ -76,10 +83,20 @@ static bool check_and_generate_certificates() {
83
}
84
85
static size_t response_write_callback(void *ptr, size_t size, size_t nmemb, void *stream) {
79
- BUFFER *wb = stream;
86
+ struct claim_response_buffer *response = stream;
87
+
88
+ if (unlikely(nmemb && size > SIZE_MAX / nmemb)) {
89
+ response->too_large = true;
90
+ return 0;
91
+ }
92
size_t real_size = size * nmemb;
93
82
- buffer_memcat(wb, ptr, real_size);
94
+ if (unlikely(real_size > CLAIM_RESPONSE_SIZE_LIMIT - buffer_strlen(response->wb))) {
95
+ response->too_large = true;
96
+ return 0;
97
+ }
98
+
99
+ buffer_memcat(response->wb, ptr, real_size);
100
101
return real_size;
102
}
@@ -154,11 +171,24 @@ static int debug_callback(CURL *handle, curl_infotype type, char *data, size_t s
171
return 0;
172
}
173
174
+static bool cleanup_curl_request_failure(CURL *curl, struct curl_slist *headers, bool *can_retry, CURLcode res,
175
+ const char *option) {
176
+ claim_agent_failure_reason_set("Cannot configure request (%s failed: %s)", option, curl_easy_strerror(res));
177
+
178
+ curl_easy_cleanup(curl);
179
+ if(headers)
180
+ curl_slist_free_all(headers);
181
+
182
+ *can_retry = false;
183
+ return false;
184
+}
185
+
186
static bool send_curl_request(const char *machine_guid, const char *hostname, const char *token, const char *rooms, const char *url, const char *proxy, bool insecure, bool *can_retry) {
187
CURL *curl;
188
CURLcode res;
189
char target_url[2048];
190
char public_key[2048] = "";
191
+ size_t public_key_bytes_read = 0;
192
FILE *fp;
193
struct curl_slist *headers = NULL;
194
@@ -175,12 +205,13 @@ static bool send_curl_request(const char *machine_guid, const char *hostname, co
205
// Read the public key
206
CLEAN_CHAR_P *public_key_file = filename_from_path_entry_strdupz(netdata_configured_cloud_dir, "public.pem");
207
fp = fopen(public_key_file, "r");
178
- if (!fp || fread(public_key, 1, sizeof(public_key), fp) == 0) {
208
+ if (!fp || (public_key_bytes_read = fread(public_key, 1, sizeof(public_key) - 1, fp)) == 0) {
209
claim_agent_failure_reason_set("cannot read public key file '%s'", public_key_file);
210
if (fp) fclose(fp);
211
*can_retry = false;
212
return false;
213
}
214
+ public_key[public_key_bytes_read] = '\0';
215
fclose(fp);
216
217
// check if we have trusted.pem
@@ -226,35 +257,57 @@ static bool send_curl_request(const char *machine_guid, const char *hostname, co
257
return false;
258
}
259
260
+#define CURL_SETOPT_OR_RETURN(option, value) \
261
+ do { \
262
+ res = curl_easy_setopt(curl, option, value); \
263
+ if(unlikely(res != CURLE_OK)) \
264
+ return cleanup_curl_request_failure(curl, headers, can_retry, res, #option); \
265
+ } while(0)
266
+
267
// curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
230
- curl_easy_setopt(curl, CURLOPT_DEBUGFUNCTION, debug_callback);
268
+ CURL_SETOPT_OR_RETURN(CURLOPT_DEBUGFUNCTION, debug_callback);
269
270
// we will receive the response in this
271
CLEAN_BUFFER *response = buffer_create(0, NULL);
272
+ struct claim_response_buffer response_buffer = {
273
+ .wb = response,
274
+ .too_large = false,
275
+ };
276
277
// configure the request
236
- headers = curl_slist_append(headers, "Content-Type: application/json");
237
- curl_easy_setopt(curl, CURLOPT_URL, target_url);
238
- curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, "PUT");
239
- curl_easy_setopt(curl, CURLOPT_POSTFIELDS, buffer_tostring(wb));
240
- curl_easy_setopt(curl, CURLOPT_HTTPHEADER, headers);
241
- curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, response_write_callback);
242
- curl_easy_setopt(curl, CURLOPT_WRITEDATA, response);
278
+ struct curl_slist *headers_with_content_type = curl_slist_append(headers, "Content-Type: application/json");
279
+ if(unlikely(!headers_with_content_type)) {
280
+ claim_agent_failure_reason_set("Cannot append Content-Type header to the claim request");
281
+ curl_easy_cleanup(curl);
282
+ if(headers)
283
+ curl_slist_free_all(headers);
284
+ *can_retry = false;
285
+ return false;
286
+ }
287
+ headers = headers_with_content_type;
288
+
289
+ CURL_SETOPT_OR_RETURN(CURLOPT_URL, target_url);
290
+ CURL_SETOPT_OR_RETURN(CURLOPT_CUSTOMREQUEST, "PUT");
291
+ CURL_SETOPT_OR_RETURN(CURLOPT_POSTFIELDS, buffer_tostring(wb));
292
+ CURL_SETOPT_OR_RETURN(CURLOPT_HTTPHEADER, headers);
293
+ CURL_SETOPT_OR_RETURN(CURLOPT_WRITEFUNCTION, response_write_callback);
294
+ CURL_SETOPT_OR_RETURN(CURLOPT_WRITEDATA, &response_buffer);
295
+ CURL_SETOPT_OR_RETURN(CURLOPT_MAXFILESIZE_LARGE, (curl_off_t)CLAIM_RESPONSE_SIZE_LIMIT);
296
297
if(trusted_key_file)
245
- curl_easy_setopt(curl, CURLOPT_CAINFO, trusted_key_file);
298
+ CURL_SETOPT_OR_RETURN(CURLOPT_CAINFO, trusted_key_file);
299
300
// Proxy configuration
301
if (proxy) {
302
if (!*proxy || strcmp(proxy, "none") == 0) {
303
// disable proxy configuration in libcurl
251
- curl_easy_setopt(curl, CURLOPT_PROXY, "");
304
+ CURL_SETOPT_OR_RETURN(CURLOPT_PROXY, "");
305
proxy = "none";
306
}
307
308
else if (strcmp(proxy, "env") != 0) {
309
// set the custom proxy for libcurl
257
- curl_easy_setopt(curl, CURLOPT_PROXY, proxy);
310
+ CURL_SETOPT_OR_RETURN(CURLOPT_PROXY, proxy);
311
}
312
313
else {
@@ -265,30 +318,46 @@ static bool send_curl_request(const char *machine_guid, const char *hostname, co
318
319
// Insecure option
320
if (insecure) {
268
- curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0L);
269
- curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0L);
321
+ CURL_SETOPT_OR_RETURN(CURLOPT_SSL_VERIFYPEER, 0L);
322
+ CURL_SETOPT_OR_RETURN(CURLOPT_SSL_VERIFYHOST, 0L);
323
}
324
325
// Set timeout options
273
- curl_easy_setopt(curl, CURLOPT_TIMEOUT, 10L);
274
- curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 5L);
326
+ CURL_SETOPT_OR_RETURN(CURLOPT_TIMEOUT, 10L);
327
+ CURL_SETOPT_OR_RETURN(CURLOPT_CONNECTTIMEOUT, 5L);
328
+
329
+#undef CURL_SETOPT_OR_RETURN
330
331
// execute the request
332
res = curl_easy_perform(curl);
333
if (res != CURLE_OK) {
279
- claim_agent_failure_reason_set("Request failed with error: %s\n"
280
- "proxy: '%s',\n"
281
- "insecure: %s,\n"
282
- "public key file: '%s',\n"
283
- "trusted key file: '%s'",
284
- curl_easy_strerror(res),
285
- proxy,
286
- insecure ? "true" : "false",
287
- public_key_file ? public_key_file : "none",
288
- trusted_key_file ? trusted_key_file : "none");
334
+ bool response_too_large = (res == CURLE_FILESIZE_EXCEEDED || response_buffer.too_large);
335
+
336
+ if (response_too_large)
337
+ claim_agent_failure_reason_set("Request failed: response body exceeded %zu bytes\n"
338
+ "proxy: '%s',\n"
339
+ "insecure: %s,\n"
340
+ "public key file: '%s',\n"
341
+ "trusted key file: '%s'",
342
+ (size_t)CLAIM_RESPONSE_SIZE_LIMIT,
343
+ proxy,
344
+ insecure ? "true" : "false",
345
+ public_key_file ? public_key_file : "none",
346
+ trusted_key_file ? trusted_key_file : "none");
347
+ else
348
+ claim_agent_failure_reason_set("Request failed with error: %s\n"
349
+ "proxy: '%s',\n"
350
+ "insecure: %s,\n"
351
+ "public key file: '%s',\n"
352
+ "trusted key file: '%s'",
353
+ curl_easy_strerror(res),
354
+ proxy,
355
+ insecure ? "true" : "false",
356
+ public_key_file ? public_key_file : "none",
357
+ trusted_key_file ? trusted_key_file : "none");
358
curl_easy_cleanup(curl);
359
curl_slist_free_all(headers);
291
- *can_retry = true;
360
+ *can_retry = !response_too_large;
361
return false;
362
}
363
@@ -319,16 +388,20 @@ static bool send_curl_request(const char *machine_guid, const char *hostname, co
388
if (json_object_object_get_ex(parsed_json, "errorMsgKey", &error_key_obj))
389
error_key = json_object_get_string(error_key_obj);
390
322
- if (strcmp(error_key, "ErrInvalidNodeID") == 0)
323
- claim_agent_failure_reason_set("Failed: the node id is invalid");
324
- else if (strcmp(error_key, "ErrInvalidNodeName") == 0)
325
- claim_agent_failure_reason_set("Failed: the node name is invalid");
326
- else if (strcmp(error_key, "ErrInvalidRoomID") == 0)
327
- claim_agent_failure_reason_set("Failed: one or more room ids are invalid");
328
- else if (strcmp(error_key, "ErrInvalidPublicKey") == 0)
329
- claim_agent_failure_reason_set("Failed: the public key is invalid");
391
+ if(error_key) {
392
+ if (strcmp(error_key, "ErrInvalidNodeID") == 0)
393
+ claim_agent_failure_reason_set("Failed: the node id is invalid");
394
+ else if (strcmp(error_key, "ErrInvalidNodeName") == 0)
395
+ claim_agent_failure_reason_set("Failed: the node name is invalid");
396
+ else if (strcmp(error_key, "ErrInvalidRoomID") == 0)
397
+ claim_agent_failure_reason_set("Failed: one or more room ids are invalid");
398
+ else if (strcmp(error_key, "ErrInvalidPublicKey") == 0)
399
+ claim_agent_failure_reason_set("Failed: the public key is invalid");
400
+ else
401
+ claim_agent_failure_reason_set("Failed with description '%s'", error_key);
402
+ }
403
else
331
- claim_agent_failure_reason_set("Failed with description '%s'", error_key);
404
+ claim_agent_failure_reason_set("Failed with a response code %ld", http_status_code);
405
406
json_object_put(parsed_json);
407
}
src/claim/claim_id.c
+1
-1
@@ -51,7 +51,7 @@ bool claim_id_set_str(const char *claim_id_str) {
51
}
52
53
ND_UUID claim_id_get_uuid(void) {
54
- static ND_UUID uuid;
54
+ ND_UUID uuid;
55
spinlock_lock(&claim.spinlock);
56
uuid = claim.claim_uuid;
57
spinlock_unlock(&claim.spinlock);