@cryptotaxi247 / netdata-1 / commits / 47c333433

Fix SonarCloud security hotspots and cgroup-network namespace handling (#22460)

* fix(json): replace unsafe sprintf with snprintfz and buffer_print_netdata_double Fix SonarCloud c:S6069 findings: - Line 106: sprintf → snprintfz for ARRAY formatting into 50-byte buffer - Line 115: Replace intermediate buffer + sprintf with buffer_print_netdata_double() which writes directly to BUFFER* and handles NaN/Inf correctly (outputs null) Security: Prevents buffer overflow with large double values (e.g., 1e308 produces 300+ chars with %f format). * fix(api): replace unsafe sprintf with snprintfz for plugin:module name Fix SonarCloud c:S6069 finding at line 18. Plugin and module names (STRING* type) have no enforced maximum length. Use snprintfz to prevent buffer overflow in the 500-byte name buffer. Security: Prevents potential buffer overflow from malicious or misconfigured collectors with long plugin/module names. * fix(cgroup-network): harden namespace switching and chroot security This commit addresses multiple security and correctness issues in the cgroup-network collector's namespace switching logic: 1. **Fix chroot escape vulnerability (CWE-243)** Added chdir("/") after successful chroot to ensure the working directory is inside the chroot jail, preventing potential escapes. 2. **Fix silent failures in namespace switching** The function previously logged errors for fchdir(), chroot(), and chdir() failures but continued execution and returned success. Now all critical filesystem operations return failure (1) on error, properly closing file descriptors. 3. **Fix overly strict namespace verification** The verification loop required ALL namespaces to succeed, causing false failures for containers with shared PID namespaces (--pid=host) or mount namespaces. Now only CLONE_NEWNET is mandatory. - CLONE_NEWNET failure: returns error (cannot detect interfaces) - CLONE_NEWNS/CLONE_NEWPID failure: logs warning, continues 4. **Proper cleanup on all error paths** All failure paths now close root_fd, cwd_fd, and namespace fds before returning. Security impact: Closes CWE-243, prevents reading host network interfaces as container data when namespace switching fails. Fixes SonarCloud hotspot c:S5802. * sow work * cgroup-network: fix chroot escape, namespace error handling, and fd cleanup * sonar-mark.sh: document family-ack in usage header * cgroup-network: use appropriate log level for optional namespace open failures * cgroup-network: fix ND_LOG_FIELD_PRIORITY type name * cgroup-network: remove unused cwd_fd variable and open * cgroup-network: remove dead assignment to root_fd after close

Costa Tsaousis committed May 20, 2026 at 11:44 UTC 47c33343384e5bd57e052269ba0b1ff82616d155

5 files changed +97 -27

.agents/skills/sonarqube-audit/scripts/sonar-mark.sh

+21

@@ -22,6 +22,7 @@

22 # FAMILY MODE (acts on every open finding for a rule):

23 # sonar-mark.sh family-fp <RULE_ID> <COMMENT> # e.g. go:S2077

24 # sonar-mark.sh family-safe <RULE_ID> <COMMENT> # e.g. c:S5443

25 +# sonar-mark.sh family-ack <RULE_ID> <COMMENT> # e.g. c:S5443

26 #

27 # Family mode prints the matched keys and prompts before acting unless

28 # SONAR_MARK_YES=1 is set in the environment.

@@ -157,6 +158,25 @@ family_safe() {

158 done <<< "${keys}"

159 }

160

161 +family_ack() {

162 + local rule="$1" comment="$2"

163 + sq_require_ascii "${comment}"

164 + local keys

165 + keys="$(list_open_hotspots_for_rule "${rule}")"

166 + local count

167 + count="$(printf '%s\n' "${keys}" | grep -c . || true)"

168 + if [[ "${count}" == "0" ]]; then

169 + echo -e "${SQ_YELLOW}No open hotspots for rule ${rule}.${SQ_NC}" >&2

170 + return 0

171 + fi

172 + echo "${keys}" >&2

173 + confirm_family "${rule}" "${count}" "mark REVIEWED/ACKNOWLEDGED" || { echo "Aborted." >&2; return 1; }

174 + while IFS= read -r key; do

175 + [[ -z "${key}" ]] && continue

176 + hotspot_change_status "${key}" "ACKNOWLEDGED" "${comment}"

177 + done <<< "${keys}"

178 +}

179 +

180 usage() {

181 sed -n '4,33p' "$0"

182 exit 2

@@ -172,6 +192,7 @@ case "${cmd}" in

192 fixed) hotspot_change_status "${1:?key required}" "FIXED" "${2:?comment required}" ;;

193 family-fp) family_fp "${1:?rule id required (e.g. go:S2077)}" "${2:?comment required}" ;;

194 family-safe) family_safe "${1:?rule id required (e.g. c:S5443)}" "${2:?comment required}" ;;

195 + family-ack) family_ack "${1:?rule id required (e.g. c:S5443)}" "${2:?comment required}" ;;

196 ""|-h|--help|help) usage ;;

197 *) echo -e "${SQ_RED}[ERROR]${SQ_NC} Unknown subcommand: ${cmd}" >&2; usage ;;

198 esac

AGENTS.md

+4 -4

@@ -328,19 +328,19 @@ Runtime input skills:

328

329 - `.agents/skills/coverity-audit/`

330 Trigger: Coverity Scan defect triage for this repository.

331 - Status: preserved under legacy name; project-skill alignment is deferred and tracked by `.agents/sow/pending/SOW-0003-20260501-legacy-runtime-skill-alignment.md`.

331 + Status: live.

332

333 - `.agents/skills/sonarqube-audit/`

334 Trigger: SonarCloud findings triage for this repository.

335 - Status: preserved under legacy name; project-skill alignment is deferred and tracked by `.agents/sow/pending/SOW-0003-20260501-legacy-runtime-skill-alignment.md`.

335 + Status: live.

336

337 - `.agents/skills/graphql-audit/`

338 Trigger: GitHub Code Scanning/CodeQL triage for this repository.

339 - Status: preserved under legacy name; project-skill alignment is deferred and tracked by `.agents/sow/pending/SOW-0003-20260501-legacy-runtime-skill-alignment.md`.

339 + Status: live.

340

341 - `.agents/skills/pr-reviews/`

342 Trigger: PR comment and review iteration work for this repository.

343 - Status: preserved under legacy name; project-skill alignment is deferred and tracked by `.agents/sow/pending/SOW-0003-20260501-legacy-runtime-skill-alignment.md`.

343 + Status: live.

344

345 - `.agents/skills/codacy-audit/`

346 Trigger: Codacy Cloud workflow for this repository -- pre-push local analysis (`codacy-analysis-cli` via docker or local binary) and read-only PR-issue fetching via the v3 API.

src/collectors/cgroups.plugin/cgroup-network.c

+69 -18

@@ -199,7 +199,7 @@ static void continue_as_child(void) {

199 exit(EXIT_FAILURE);

200 }

201

202 -int proc_pid_fd(const char *prefix, const char *ns, pid_t pid) {

202 +int proc_pid_fd(const char *prefix, const char *ns, pid_t pid, ND_LOG_FIELD_PRIORITY priority) {

203 if(!prefix) prefix = "";

204

205 char filename[FILENAME_MAX + 1];

@@ -207,7 +207,7 @@ int proc_pid_fd(const char *prefix, const char *ns, pid_t pid) {

207 int fd = open(filename, O_RDONLY | O_CLOEXEC);

208

209 if(fd == -1)

210 - nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot open proc_pid_fd() file '%s'", filename);

210 + nd_log(NDLS_COLLECTORS, priority, "Cannot open proc_pid_fd() file '%s'", filename);

211

212 return fd;

213 }

@@ -234,11 +234,27 @@ static struct ns {

234 static int switch_namespace(const char *prefix, pid_t pid) {

235 #ifdef HAVE_SETNS

236 int i;

237 - for(i = 0; all_ns[i].name ; i++)

238 - all_ns[i].fd = proc_pid_fd(prefix, all_ns[i].path, pid);

237 + int root_fd = -1;

238 +

239 + for(i = 0; all_ns[i].name ; i++) {

240 + // Only network namespace is mandatory; optional namespaces log warnings

241 + ND_LOG_FIELD_PRIORITY prio = (all_ns[i].nstype == CLONE_NEWNET) ? NDLP_ERR : NDLP_WARNING;

242 + all_ns[i].fd = proc_pid_fd(prefix, all_ns[i].path, pid, prio);

243 + }

244

240 - int root_fd = proc_pid_fd(prefix, "root", pid);

241 - int cwd_fd = proc_pid_fd(prefix, "cwd", pid);

245 + root_fd = proc_pid_fd(prefix, "root", pid, NDLP_ERR);

246 +

247 + // Verify we can access the network namespace fd

248 + // This is the only namespace critical for correct interface detection

249 + for(i = 0; all_ns[i].name ; i++) {

250 + if(all_ns[i].nstype == CLONE_NEWNET) {

251 + if(all_ns[i].fd == -1) {

252 + // proc_pid_fd() already logs the open failure

253 + goto cleanup_and_fail;

254 + }

255 + break;

256 + }

257 + }

258

259 setgroups(0, NULL);

260

@@ -255,9 +271,13 @@ static int switch_namespace(const char *prefix, pid_t pid) {

271 if(setns(all_ns[i].fd, all_ns[i].nstype) == -1) {

272 if(pass == 1) {

273 all_ns[i].status = 0;

258 - nd_log(NDLS_COLLECTORS, NDLP_ERR,

259 - "Cannot switch to %s namespace of pid %d",

260 - all_ns[i].name, (int) pid);

274 + // Only log critical namespace failures here;

275 + // non-critical failures are logged in the verification loop below

276 + if(all_ns[i].nstype == CLONE_NEWNET) {

277 + nd_log(NDLS_COLLECTORS, NDLP_ERR,

278 + "Cannot switch to %s namespace of pid %d",

279 + all_ns[i].name, (int) pid);

280 + }

281 }

282 }

283 else

@@ -266,24 +286,43 @@ static int switch_namespace(const char *prefix, pid_t pid) {

286 }

287 }

288

289 + // Verify critical namespaces were successfully switched

290 + for(i = 0; all_ns[i].name ; i++) {

291 + if(all_ns[i].fd != -1 && !all_ns[i].status) {

292 + if(all_ns[i].nstype == CLONE_NEWNET) {

293 + // Network namespace is mandatory for correct interface detection

294 + nd_log(NDLS_COLLECTORS, NDLP_ERR,

295 + "Failed to switch to %s namespace of pid %d",

296 + all_ns[i].name, (int) pid);

297 + goto cleanup_and_fail;

298 + }

299 + // Mount/PID namespace failure is non-critical for network detection

300 + nd_log(NDLS_COLLECTORS, NDLP_WARNING,

301 + "Failed to switch to %s namespace of pid %d (continuing)",

302 + all_ns[i].name, (int) pid);

303 + }

304 + }

305 +

306 gettid_uncached();

307 setgroups(0, NULL);

308

309 if(root_fd != -1) {

273 - if(fchdir(root_fd) < 0)

310 + if(fchdir(root_fd) < 0) {

311 nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot fchdir() to pid %d root directory", (int)pid);

312 + goto cleanup_and_fail;

313 + }

314

276 - if(chroot(".") < 0)

315 + if(chroot(".") < 0) {

316 nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot chroot() to pid %d root directory", (int)pid);

317 + goto cleanup_and_fail;

318 + }

319

279 - close(root_fd);

280 - }

281 -

282 - if(cwd_fd != -1) {

283 - if(fchdir(cwd_fd) < 0)

284 - nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot fchdir() to pid %d current working directory", (int)pid);

320 + if(chdir("/") < 0) {

321 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot chdir() to / after chroot for pid %d", (int)pid);

322 + goto cleanup_and_fail;

323 + }

324

286 - close(cwd_fd);

325 + close(root_fd);

326 }

327

328 int do_fork = 0;

@@ -295,6 +334,7 @@ static int switch_namespace(const char *prefix, pid_t pid) {

334 do_fork = 1;

335

336 close(all_ns[i].fd);

337 + all_ns[i].fd = -1;

338 }

339

340 if(do_fork)

@@ -302,6 +342,17 @@ static int switch_namespace(const char *prefix, pid_t pid) {

342

343 return 0;

344

345 +cleanup_and_fail:

346 + if(root_fd != -1) close(root_fd);

347 + for(i = 0; all_ns[i].name ; i++) {

348 + if(all_ns[i].fd != -1) {

349 + close(all_ns[i].fd);

350 + all_ns[i].fd = -1;

351 + }

352 + all_ns[i].status = -1;

353 + }

354 + return 1;

355 +

356 #else

357

358 errno = ENOSYS;

src/libnetdata/json/json.c

+2 -4

@@ -103,7 +103,7 @@ int json_callback_print(JSON_ENTRY *e)

103

104 case JSON_ARRAY:

105 e->callback_function = json_callback_print;

106 - sprintf(txt,"ARRAY[%lu]", (long unsigned int) e->data.items);

106 + snprintfz(txt, sizeof(txt), "ARRAY[%lu]", (long unsigned int) e->data.items);

107 buffer_strcat(wb, txt);

108 break;

109

@@ -112,9 +112,7 @@ int json_callback_print(JSON_ENTRY *e)

112 break;

113

114 case JSON_NUMBER:

115 - sprintf(txt, NETDATA_DOUBLE_FORMAT_AUTO, e->data.number);

116 - buffer_strcat(wb,txt);

117 -

115 + buffer_print_netdata_double(wb, e->data.number);

116 break;

117

118 case JSON_BOOLEAN:

src/web/api/v1/api_v1_info.c

+1 -1

@@ -15,7 +15,7 @@ static void host_collectors(RRDHOST *host, BUFFER *wb) {

15 if (!rrdset_is_available_for_viewers(st))

16 continue;

18 - sprintf(name, "%s:%s", rrdset_plugin_name(st), rrdset_module_name(st));

18 + snprintfz(name, sizeof(name), "%s:%s", rrdset_plugin_name(st), rrdset_module_name(st));

20 bool old = 0;

21 bool *set = dictionary_set(dict, name, &old, sizeof(bool));