Fix based on Coverity and Sonar audits (part 3) - #22331
Merged
Merged
Conversation
Sonar c:S2612 (MAJOR vulnerability): the management API key file was created with `open(..., O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 444)`. The literal `444` is decimal, not octal, which equals octal 0674 = rw-rwxr--. Group gets read+write+execute and others get read on a file that stores the management API key (a UUID granting admin endpoint access). Anyone with local read on the host can lift the key. Even if the original intent was octal `0444` (world-readable), a secret-key file should not be readable by group or others. Use `0600` (owner read+write only), the standard mode for secrets.
Sonar c:S2612: file_lock_get() created the advisory lock file with mode 0666 (rw-rw-rw-). Netdata runs as a single dedicated user, so group/other access is unnecessary and adds latent risk if any local user can interfere with the lock file. flock(2) is purely advisory and does not enforce by mode, but the open() permission still controls who can create/access the file. Owner-only 0600 keeps the locking behavior intact for the netdata user while preventing unrelated local users from creating or opening the file. The function is currently unused (its caller in src/daemon/main.c is commented out), but it is exposed in the public header and may be revived for single-instance enforcement; tightening the default now avoids carrying permissive bits forward.
… -> 0666) Sonar c:S2612: create_listen_socket_unix() chmod'd the bound UNIX socket file to 0777. For UNIX domain socket files only the read/write permissions affect client connect() access -- the execute bit is unused. 0777 and 0666 are functionally identical for socket connect. Both callers (web API and statsd) intentionally allow arbitrary local clients to connect, so the broad rw permission is preserved with 0666. The execute bit was misleading and unnecessary; remove it and update the explanatory comment.
|
stelfrag
marked this pull request as ready for review
April 30, 2026 14:23
stelfrag
marked this pull request as draft
April 30, 2026 14:23
Collaborator
Author
|
@cubic-dev-ai review this PR |
Contributor
@stelfrag I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
stelfrag
marked this pull request as ready for review
May 2, 2026 19:58
thiagoftsm
approved these changes
May 6, 2026
thiagoftsm
left a comment
Contributor
There was a problem hiding this comment.
PR is working as expected on Linux. LGTM!
Merged
Ferroin
pushed a commit
that referenced
this pull request
Jul 15, 2026
* api: tighten management API key file permissions to 0600 Sonar c:S2612 (MAJOR vulnerability): the management API key file was created with `open(..., O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 444)`. The literal `444` is decimal, not octal, which equals octal 0674 = rw-rwxr--. Group gets read+write+execute and others get read on a file that stores the management API key (a UUID granting admin endpoint access). Anyone with local read on the host can lift the key. Even if the original intent was octal `0444` (world-readable), a secret-key file should not be readable by group or others. Use `0600` (owner read+write only), the standard mode for secrets. * file_lock: tighten lock file creation mode from 0666 to 0600 Sonar c:S2612: file_lock_get() created the advisory lock file with mode 0666 (rw-rw-rw-). Netdata runs as a single dedicated user, so group/other access is unnecessary and adds latent risk if any local user can interfere with the lock file. flock(2) is purely advisory and does not enforce by mode, but the open() permission still controls who can create/access the file. Owner-only 0600 keeps the locking behavior intact for the netdata user while preventing unrelated local users from creating or opening the file. The function is currently unused (its caller in src/daemon/main.c is commented out), but it is exposed in the public header and may be revived for single-instance enforcement; tightening the default now avoids carrying permissive bits forward. * listen-sockets: drop misleading exec bit from UNIX socket chmod (0777 -> 0666) Sonar c:S2612: create_listen_socket_unix() chmod'd the bound UNIX socket file to 0777. For UNIX domain socket files only the read/write permissions affect client connect() access -- the execute bit is unused. 0777 and 0666 are functionally identical for socket connect. Both callers (web API and statsd) intentionally allow arbitrary local clients to connect, so the broad rw permission is preserved with 0666. The execute bit was misleading and unnecessary; remove it and update the explanatory comment. --------- Co-authored-by: Costa Tsaousis <costa@netdata.cloud> (cherry picked from commit d6029b8)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Summary