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

stelfrag · 2026-04-24T18:22:07Z

Summary

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

Summary by cubic

Fixes memory-safety and correctness issues flagged by Coverity in log2journal and the nofork spawn server, preventing out-of-bounds reads and rejecting invalid AF_UNIX socket paths. Adds a test for trailing backslashes in logfmt and improves error logs.

Bug Fixes
- log2journal (logfmt): treat a trailing backslash in keys/values as a literal so parsing stays in-bounds; minor quote check cleanup.
- log2journal (PCRE2): index journal_key_characters_map with (unsigned char) to avoid large indexes on non-ASCII bytes.
- spawn server (nofork): validate AF_UNIX socket path length before use; use bounded copies when setting sun_path; detect and fail on snprintf truncation; fix an error log message.
- Tests: add logfmt-trailing-backslash case to lock behavior.

^{Written for commit 023dc8f. Summary will update on new commits.}

Coverity CID 410102 (TAINTED_SCALAR): copy_and_convert_key() cast PCRE2 name-table bytes to unsigned instead of unsigned char, which can turn non-ASCII UTF bytes in named groups into huge indexes on signed-char builds. Cast through unsigned char before indexing journal_key_characters_map.

Coverity CID 410217 (TAINTED_SCALAR): logfmt escape parsing advanced past the terminating NUL when a key or value ended with a lone backslash. Treat trailing backslashes as literal characters so the parser stays in-bounds without changing valid escape handling.

Coverity CID 439982 (STRING_OVERFLOW): reject AF_UNIX socket paths that do not fit in sun_path before the nofork spawn server stores or uses them. The nofork connect and bind paths now use bounded copies after that validation, fixing both copies of the same root cause.

Fail with an explicit error message when the formatted socket path would not fit in the local buffer. This catches the truncation case before the AF_UNIX length check and produces a more actionable log line.

cubic-dev-ai

No issues found across 6 files

Confidence score: 5/5

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

Architecture diagram

sequenceDiagram
    participant LogSrc as Log Source
    participant L2J as log2journal (logfmt/PCRE2)
    participant Map as journal_key_characters_map
    participant Parent as Parent Process
    participant SpawnSrv as Spawn Server (nofork)
    participant OS as OS (Filesystem/Sockets)

    Note over LogSrc, Map: Log Parsing Flow (Memory Safety Fixes)

    LogSrc->>L2J: Provide log line (e.g. key=value\)
    L2J->>L2J: logftm_parse_value()
    opt NEW: Trailing backslash detected
        L2J->>L2J: Treat '\' as literal char (prevent OOB read)
    end

    L2J->>Map: Lookup character index
    Note right of Map: CHANGED: Indexing now uses (unsigned char)<br/>to prevent sign-extension/large offsets.
    Map-->>L2J: Validated Journal Key char

    Note over Parent, OS: Spawn Server Lifecycle (Validation Fixes)

    Parent->>SpawnSrv: spawn_server_create(name)
    SpawnSrv->>SpawnSrv: Generate socket path (snprintf)
    
    alt NEW: Path exceeds buffer or AF_UNIX limit
        SpawnSrv-->>Parent: Return NULL (Log truncation/length error)
    else Path is valid
        SpawnSrv->>OS: unlink(path)
        SpawnSrv->>OS: NEW: spawn_server_set_unix_socket_path()
        Note right of OS: Uses bounded strncpyz & length checks
        SpawnSrv->>OS: socket(AF_UNIX) & bind()
        SpawnSrv-->>Parent: Server Handle
    end

    Parent->>SpawnSrv: connect_to_spawn_server(path)
    SpawnSrv->>SpawnSrv: NEW: validate path length < sun_path
    alt NEW: Invalid Path Length
        SpawnSrv-->>Parent: Return -1 (ENAMETOOLONG)
    else Valid
        SpawnSrv->>OS: connect(sock, addr)
        SpawnSrv-->>Parent: Connection FD
    end

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Addresses memory-safety and correctness findings from a Coverity audit by hardening AF_UNIX socket path handling in the nofork spawn server and fixing log2journal parsing edge-cases (including trailing backslashes), plus adding a regression test.

Changes:

Add AF_UNIX socket path length validation and bounded copies for spawn server connect/listen and socket path generation.
Fix logfmt parsing to treat trailing backslashes as literals (avoid out-of-bounds reads) and tighten quote handling.
Prevent out-of-range indexing in PCRE2 key conversion by casting input bytes to unsigned char; add a logfmt regression test case.

Reviewed changes

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

Show a summary per file

File	Description
src/libnetdata/spawn_server/spawn_server_nofork.c	Introduces bounded AF_UNIX socket-path handling and improved path-generation error handling.
src/collectors/log2journal/tests.d/logfmt-trailing-backslash.yaml	Adds a new logfmt test case descriptor.
src/collectors/log2journal/tests.d/logfmt-trailing-backslash.input	Adds input fixture covering trailing backslash behavior.
src/collectors/log2journal/tests.d/logfmt-trailing-backslash.output	Adds expected output fixture for trailing backslash behavior.
src/collectors/log2journal/log2journal-pcre2.c	Avoids invalid indexing by casting key bytes to `unsigned char`.
src/collectors/log2journal/log2journal-logfmt.c	Fixes edge-case parsing to keep reads in-bounds for trailing backslashes in keys/values.

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

sonarqubecloud · 2026-04-24T18:41:14Z

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

Copilot

Pull request overview

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

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

thiagoftsm

PR is running as expected. No issues found during runtime. LGTM!

…rt 4) (#22270) * log2journal: fix utf named-group key indexing Coverity CID 410102 (TAINTED_SCALAR): copy_and_convert_key() cast PCRE2 name-table bytes to unsigned instead of unsigned char, which can turn non-ASCII UTF bytes in named groups into huge indexes on signed-char builds. Cast through unsigned char before indexing journal_key_characters_map. * log2journal: handle trailing logfmt escapes safely Coverity CID 410217 (TAINTED_SCALAR): logfmt escape parsing advanced past the terminating NUL when a key or value ended with a lone backslash. Treat trailing backslashes as literal characters so the parser stays in-bounds without changing valid escape handling. * spawn_server: reject oversized unix socket paths Coverity CID 439982 (STRING_OVERFLOW): reject AF_UNIX socket paths that do not fit in sun_path before the nofork spawn server stores or uses them. The nofork connect and bind paths now use bounded copies after that validation, fixing both copies of the same root cause. * spawn_server: detect snprintf truncation of socket path Fail with an explicit error message when the formatted socket path would not fit in the local buffer. This catches the truncation case before the AF_UNIX length check and produces a more actionable log line. * Fix log message --------- Co-authored-by: Costa Tsaousis <costa@netdata.cloud> (cherry picked from commit d3139a8)

ktsaou added 4 commits April 24, 2026 20:56

spawn_server: detect snprintf truncation of socket path

498253d

Fail with an explicit error message when the formatted socket path would not fit in the local buffer. This catches the truncation case before the AF_UNIX length check and produces a more actionable log line.

cubic-dev-ai Bot reviewed Apr 24, 2026

View reviewed changes

stelfrag marked this pull request as ready for review April 24, 2026 18:25

stelfrag requested a review from thiagoftsm as a code owner April 24, 2026 18:25

Copilot AI review requested due to automatic review settings April 24, 2026 18:25

stelfrag requested a review from vkalintiris as a code owner April 24, 2026 18:25

stelfrag marked this pull request as draft April 24, 2026 18:26

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Comment thread src/libnetdata/spawn_server/spawn_server_nofork.c Outdated

Comment thread src/libnetdata/spawn_server/spawn_server_nofork.c

github-actions Bot added the area/collectors Everything related to data collection label Apr 24, 2026

Fix log message

023dc8f

Copilot started reviewing on behalf of stelfrag April 24, 2026 18:41 View session

stelfrag requested a review from Copilot April 24, 2026 18:46

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Comment thread src/libnetdata/spawn_server/spawn_server_nofork.c

Comment thread src/libnetdata/spawn_server/spawn_server_nofork.c

Copilot started reviewing on behalf of stelfrag April 24, 2026 19:03 View session

stelfrag marked this pull request as ready for review April 24, 2026 19:40

thiagoftsm approved these changes Apr 24, 2026

View reviewed changes

stelfrag merged commit d3139a8 into netdata:master Apr 25, 2026
162 checks passed

stelfrag deleted the cov_fix_part4 branch April 25, 2026 13:06

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 4) - #22270

Fix memory-safety and correctness bugs surfaced by Coverity audit (part 4)#22270
stelfrag merged 5 commits into
netdata:masterfrom
stelfrag:cov_fix_part4

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!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Apr 24, 2026

Quality Gate passed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

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