Fix memory-safety and correctness bugs surfaced by Coverity audit (part 2) by stelfrag · Pull Request #22267 · netdata/netdata

stelfrag · 2026-04-24T16:38:14Z

Summary

Split / based on Fix memory-safety and correctness bugs surfaced by Coverity audit #22234

Summary by cubic

Hardened the claim HTTP flow and UUID getter by fixing memory-safety, overflow, and error-handling issues from a Coverity audit. This prevents oversized responses, avoids NULL/overflow bugs, and improves retry behavior.

Bug Fixes
- Capped claim response to 10 MiB; abort on overflow and on size*nmemb multiplication overflow; set libcurl max response size.
- Fail fast on curl_easy_setopt() errors and on curl_slist_append() allocation failure; clean up and set can_retry=false.
- Read public.pem safely: read at most sizeof-1, track bytes read, and NUL-terminate.
- Guard 422 parsing when errorMsgKey is missing; fall back to a generic message using the HTTP status.
- Remove shared static from claim_id_get_uuid() to avoid races; use a stack-local UUID.
- Do not retry when the response is too large or request setup fails; keep retries for transient transfer errors.

^{Written for commit 9d5f13f. Summary will update on new commits.}

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.

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.

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.

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.

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.

`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.

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.

cubic-dev-ai

No issues found across 2 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

Architecture diagram

sequenceDiagram
    participant Node as Netdata Node
    participant Claim as Claiming Service
    participant FS as Local Filesystem
    participant CURL as libcurl
    participant Cloud as Cloud API (PUT /claim)

    Note over Claim, Cloud: Node Claiming Flow with Hardened Safety

    Node->>Claim: Initiate Claim
    Claim->>Claim: CHANGED: claim_id_get_uuid()<br/>(Now uses stack-local UUID to avoid races)
    
    Claim->>FS: Open public.pem
    FS-->>Claim: Key data
    Claim->>Claim: CHANGED: Read max sizeof-1 and NUL-terminate

    Claim->>CURL: NEW: Initialize request with strict checks
    alt Header or Option Setup Fails
        Claim->>Claim: NEW: cleanup_curl_request_failure()<br/>(Set can_retry = false)
    else Success
        Claim->>CURL: Set CURL_SETOPT (URL, Auth, Headers)
        Claim->>CURL: NEW: Set CURLOPT_MAXFILESIZE_LARGE (10 MiB)
    end

    Claim->>Cloud: PUT claim request
    
    loop While Data Received
        Cloud-->>CURL: Data chunk
        CURL->>Claim: response_write_callback()
        alt NEW: Size > 10 MiB OR integer overflow
            Claim-->>CURL: Return 0 (Abort)
            Note right of Claim: response->too_large = true
        else Within limits
            Claim->>Claim: buffer_memcat()
            Claim-->>CURL: Return real_size
        end
    end

    alt Transfer Success (2xx)
        CURL-->>Claim: CURLE_OK
        Claim->>Node: Claim Successful
    else Transfer Failure or Overflow
        CURL-->>Claim: CURLE_FILESIZE_EXCEEDED or other error
        alt NEW: Response too large
            Claim->>Claim: Set can_retry = false
        else Transient Network Error
            Claim->>Claim: Set can_retry = true
        end
        Claim->>Node: Failure (with detailed reason)
    end

    opt HTTP 422 Unprocessable Entity
        Claim->>Claim: CHANGED: Parse JSON errorMsgKey
        Note right of Claim: Fallback to HTTP status if key missing
    end

Copilot

Pull request overview

Hardens the Netdata Cloud claiming path by addressing memory-safety and correctness issues in the claim HTTP flow and claim UUID retrieval.

Changes:

Make claim_id_get_uuid() return a stack-local UUID copy (avoids shared static state).
Add bounded response buffering (10 MiB cap) with overflow protection in the libcurl write callback.
Fail fast on libcurl request setup errors (setopt/slist) and improve handling of missing errorMsgKey in HTTP 422 responses; safely NUL-terminate public.pem reads.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
`src/claim/claim_id.c`	Removes a function-local static UUID temporary to avoid shared state and keep the getter purely stack-local.
`src/claim/claim-with-api.c`	Adds response-size limits/overflow guards, checks curl setup failures, safely terminates public key reads, and hardens 422 error parsing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

sonarqubecloud · 2026-04-24T16:50:24Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

thiagoftsm

No issue found during runtime (Coredump or communication with Cloud). LGTM!

…rt 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> (cherry picked from commit 8b118b6)

ktsaou added 7 commits April 24, 2026 19:29

claim: guard missing 422 errorMsgKey

3f8258e

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: guard size * nmemb overflow in response write callback

ac7f5fb

`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.

github-actions Bot added the area/claim label Apr 24, 2026

stelfrag marked this pull request as ready for review April 24, 2026 16:39

stelfrag requested a review from Ferroin as a code owner April 24, 2026 16:39

Copilot AI review requested due to automatic review settings April 24, 2026 16:39

stelfrag marked this pull request as draft April 24, 2026 16:39

stelfrag requested a review from thiagoftsm April 24, 2026 16:39

cubic-dev-ai Bot reviewed Apr 24, 2026

View reviewed changes

Copilot AI reviewed Apr 24, 2026

View reviewed changes

stelfrag marked this pull request as ready for review April 24, 2026 16:59

thiagoftsm approved these changes Apr 24, 2026

View reviewed changes

stelfrag merged commit 8b118b6 into netdata:master Apr 25, 2026
219 of 222 checks passed

stelfrag deleted the cov_fix_part2 branch April 25, 2026 13:02

This was referenced Apr 27, 2026

Fix memory-safety and correctness bugs surfaced by Coverity audit (part 8) #22293

Merged

Fix memory-safety and correctness bugs surfaced by Coverity audit #22234

Closed

stelfrag mentioned this pull request Jun 22, 2026

Patch release 2.10.4 #22825

Merged

Navigation Menu

Uh oh!

Fix memory-safety and correctness bugs surfaced by Coverity audit (part 2) - #22267

Fix memory-safety and correctness bugs surfaced by Coverity audit (part 2)#22267
stelfrag merged 7 commits into
netdata:masterfrom
stelfrag:cov_fix_part2

stelfrag commented Apr 24, 2026 •

edited by cubic-dev-ai Bot

Loading

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

sonarqubecloud Bot commented Apr 24, 2026

Uh oh!

thiagoftsm left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

stelfrag commented Apr 24, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Summary by cubic

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

sonarqubecloud Bot commented Apr 24, 2026

Quality Gate passed

Uh oh!

thiagoftsm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

stelfrag commented Apr 24, 2026 •

edited by cubic-dev-ai Bot

Loading