Fix memory-safety and correctness bugs surfaced by Coverity audit (part 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>
Stelios Fragkakis committed
Apr 25, 2026 at 16:05 UTC
d3139a8bd1282165e245e05e550cd44d809b14a3
6 files changed
+102
-36
src/collectors/log2journal/log2journal-logfmt.c
+34
-24
@@ -53,7 +53,7 @@ static inline bool logftm_parse_value(LOGFMT_STATE *lfs) {
53
54
char quote = '\0';
55
const char *s = logfmt_current_pos(lfs);
56
- if(*s == '\"' || *s == '\'') {
56
+ if(*s == '"' || *s == '\'') {
57
quote = *s;
58
logfmt_consume_char(lfs);
59
}
@@ -70,27 +70,31 @@ static inline bool logftm_parse_value(LOGFMT_STATE *lfs) {
70
if (*s == '\\') {
71
s++;
72
73
- switch (*s) {
74
- case 'n':
75
- copy_newline(lfs, &d, &remaining);
76
- s++;
77
- continue;
78
-
79
- case 't':
80
- copy_tab(lfs, &d, &remaining);
81
- s++;
82
- continue;
83
-
84
- case 'f':
85
- case 'b':
86
- case 'r':
87
- c = ' ';
88
- s++;
89
- break;
90
-
91
- default:
92
- c = *s++;
93
- break;
73
+ if(!*s)
74
+ c = '\\';
75
+ else {
76
+ switch (*s) {
77
+ case 'n':
78
+ copy_newline(lfs, &d, &remaining);
79
+ s++;
80
+ continue;
81
+
82
+ case 't':
83
+ copy_tab(lfs, &d, &remaining);
84
+ s++;
85
+ continue;
86
+
87
+ case 'f':
88
+ case 'b':
89
+ case 'r':
90
+ c = ' ';
91
+ s++;
92
+ break;
93
+
94
+ default:
95
+ c = *s++;
96
+ break;
97
+ }
98
}
99
}
100
else
@@ -140,10 +144,16 @@ static inline bool logfmt_parse_key(LOGFMT_STATE *lfs) {
144
while(*s && *s != '=') {
145
char c;
146
143
- if (*s == '\\')
147
+ if (*s == '\\') {
148
s++;
149
146
- c = journal_key_characters_map[(unsigned char)*s++];
150
+ if(!*s)
151
+ c = journal_key_characters_map[(unsigned char)'\\'];
152
+ else
153
+ c = journal_key_characters_map[(unsigned char)*s++];
154
+ }
155
+ else
156
+ c = journal_key_characters_map[(unsigned char)*s++];
157
158
if(c == '_' && last_c == '_')
159
continue;
src/collectors/log2journal/log2journal-pcre2.c
+1
-1
@@ -24,7 +24,7 @@ static inline void copy_and_convert_key(PCRE2_STATE *pcre2, const char *key) {
24
size_t remaining = sizeof(pcre2->key) - pcre2->key_start;
25
26
while(remaining >= 2 && *key) {
27
- *d = journal_key_characters_map[(unsigned) (*key)];
27
+ *d = journal_key_characters_map[(unsigned char)*key];
28
remaining--;
29
key++;
30
d++;
src/collectors/log2journal/tests.d/logfmt-trailing-backslash.input
new
+1
@@ -0,0 +1 @@
1
+key=value\
src/collectors/log2journal/tests.d/logfmt-trailing-backslash.output
new
+2
@@ -0,0 +1,2 @@
1
+KEY=value\
2
+
src/collectors/log2journal/tests.d/logfmt-trailing-backslash.yaml
new
+1
@@ -0,0 +1 @@
1
+pattern: logfmt
src/libnetdata/spawn_server/spawn_server_nofork.c
+63
-11
@@ -28,10 +28,38 @@ static volatile bool spawn_server_exit = false;
28
static volatile bool spawn_server_sigchld = false;
29
static SPAWN_REQUEST *spawn_server_requests = NULL;
30
31
+static size_t spawn_server_max_unix_socket_path_length(void) {
32
+ struct sockaddr_un server_addr = { 0 };
33
+ return sizeof(server_addr.sun_path) - 1;
34
+}
35
+
36
+static bool spawn_server_set_unix_socket_path(struct sockaddr_un *server_addr, const char *path, bool log, const char *action) {
37
+ const size_t max_path_length = sizeof(server_addr->sun_path) - 1;
38
+
39
+ if(strlen(path) > max_path_length) {
40
+ errno = ENAMETOOLONG;
41
+ if(log)
42
+ nd_log(NDLS_COLLECTORS, NDLP_ERR,
43
+ "%s '%s': exceeds the %zu-byte AF_UNIX limit",
44
+ action, path, max_path_length);
45
+ return false;
46
+ }
47
+
48
+ strncpyz(server_addr->sun_path, path, max_path_length);
49
+ return true;
50
+}
51
+
52
// --------------------------------------------------------------------------------------------------------------------
53
54
static int connect_to_spawn_server(const char *path, bool log) {
55
int sock = -1;
56
+ struct sockaddr_un server_addr = {
57
+ .sun_family = AF_UNIX,
58
+ };
59
+
60
+ if(!spawn_server_set_unix_socket_path(&server_addr, path, log,
61
+ "SPAWN PARENT: Cannot connect() to spawn server on path"))
62
+ return -1;
63
64
if ((sock = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
65
if(log)
@@ -39,11 +67,6 @@ static int connect_to_spawn_server(const char *path, bool log) {
67
return -1;
68
}
69
42
- struct sockaddr_un server_addr = {
43
- .sun_family = AF_UNIX,
44
- };
45
- strcpy(server_addr.sun_path, path);
46
-
70
if (connect(sock, (struct sockaddr *)&server_addr, sizeof(server_addr)) == -1) {
71
if(log)
72
nd_log(NDLS_COLLECTORS, NDLP_ERR, "SPAWN PARENT: Cannot connect() to spawn server on path '%s'.", path);
@@ -956,15 +979,19 @@ static bool spawn_server_create_listening_socket(SPAWN_SERVER *server) {
979
return false;
980
}
981
982
+ struct sockaddr_un server_addr = {
983
+ .sun_family = AF_UNIX,
984
+ };
985
+
986
+ if(!spawn_server_set_unix_socket_path(&server_addr, server->path, true,
987
+ "SPAWN SERVER: Cannot listen on path"))
988
+ return false;
989
+
990
if ((server->sock = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
991
nd_log(NDLS_COLLECTORS, NDLP_ERR, "SPAWN SERVER: Failed to create socket()");
992
return false;
993
}
994
964
- struct sockaddr_un server_addr = {
965
- .sun_family = AF_UNIX,
966
- };
967
- strcpy(server_addr.sun_path, server->path);
995
unlink(server->path);
996
errno = 0;
997
@@ -1053,13 +1080,38 @@ SPAWN_SERVER* spawn_server_create(SPAWN_SERVER_OPTIONS options, const char *name
1080
runtime_directory = "/tmp";
1081
1082
char path[1024];
1083
+ int path_length;
1084
+ const size_t max_path_length = spawn_server_max_unix_socket_path_length();
1085
if(name && *name) {
1086
server->name = strdupz(name);
1058
- snprintf(path, sizeof(path), "%s/netdata-spawn-%s.sock", runtime_directory, name);
1087
+ path_length = snprintf(path, sizeof(path), "%s/netdata-spawn-%s.sock", runtime_directory, name);
1088
}
1089
else {
1090
server->name = strdupz("unnamed");
1062
- snprintf(path, sizeof(path), "%s/netdata-spawn-%d-%zu.sock", runtime_directory, getpid(), server->id);
1091
+ path_length = snprintf(path, sizeof(path), "%s/netdata-spawn-%d-%zu.sock", runtime_directory, getpid(), server->id);
1092
+ }
1093
+
1094
+ if(path_length < 0) {
1095
+ nd_log(NDLS_COLLECTORS, NDLP_ERR,
1096
+ "SPAWN SERVER: failed to generate socket path for '%s'",
1097
+ server->name);
1098
+ goto cleanup;
1099
+ }
1100
+
1101
+ if((size_t)path_length >= sizeof(path)) {
1102
+ errno = ENAMETOOLONG;
1103
+ nd_log(NDLS_COLLECTORS, NDLP_ERR,
1104
+ "SPAWN SERVER: socket path for '%s' in runtime directory '%s' was truncated (needed %d chars plus NUL, buffer is %zu bytes)",
1105
+ server->name, runtime_directory, path_length, sizeof(path));
1106
+ goto cleanup;
1107
+ }
1108
+
1109
+ if((size_t)path_length > max_path_length) {
1110
+ errno = ENAMETOOLONG;
1111
+ nd_log(NDLS_COLLECTORS, NDLP_ERR,
1112
+ "SPAWN SERVER: socket path for '%s' in runtime directory '%s' exceeds the %zu-byte AF_UNIX limit",
1113
+ server->name, runtime_directory, max_path_length);
1114
+ goto cleanup;
1115
}
1116
1117
server->path = strdupz(path);