Harden netipc string copies

Author: ktsaou

.agents/sow/current/SOW-0015-20260605-codacy-scope-and-maintainability.md

@@ -593,6 +593,30 @@ Validation completed for this follow-up:
 - Win11 temp-copy Rust validation: `cargo test --manifest-path src/crates/netipc/Cargo.toml service::raw -- --test-threads=1`: 22 Windows raw-service tests passed.
 - Win11 temp-copy Go validation: `cd src/go && CGO_ENABLED=0 go test -count=1 ./pkg/netipc/service/raw ./pkg/netipc/transport/windows`: passed.
 
+### 2026-06-07 - PR 22649 SonarCloud Hotspot Follow-up
+
+Pre-push Netdata PR #22649 sync found nine SonarCloud security hotspots on the old PR head:
+
+- `src/libnetdata/netipc/src/transport/posix/netipc_uds_lifecycle.c`: four `strncpy` hotspot reports around socket path copies.
+- `src/libnetdata/netipc/src/service/netipc_service_common.c`: one `strlen` hotspot in service context field copying.
+- `src/libnetdata/netipc/src/service/netipc_service_posix_server.c`: two `strlen` hotspots in server config field copying.
+- `src/libnetdata/netipc/src/service/netipc_service_win_server.c`: two `strlen` hotspots in Windows server config field copying.
+
+Implemented SDK follow-up:
+
+- Replaced POSIX UDS `strncpy` socket/path copies with a checked NUL-terminated bounded copy helper and a `sockaddr_un` fill helper.
+- Replaced repeated POSIX and Windows service server `strlen`/`memcpy` config-field copies with `nipc_service_common_copy_cstr_field()`.
+- Changed the common service copy helper to avoid open-ended `strlen` while preserving previous truncating field-copy behavior.
+- Same-pattern search found two additional `strncpy` copies in POSIX SHM context path storage; those were changed to checked bounded copies in the same increment.
+
+Validation completed for this hotspot follow-up:
+
+- `git diff --check`: passed.
+- `rg "strncpy\\(|strlen\\(run_dir\\)|strlen\\(service_name\\)|size_t len = strlen" src/libnetdata/netipc/src src/libnetdata/netipc/include`: only the unrelated Windows named-pipe hash input remains.
+- `cmake --build build`: passed.
+- `/usr/bin/ctest --test-dir build --output-on-failure`: 46/46 tests passed.
+- Win11 temp-copy focused C validation: MSYS CMake build of `test_win_service`, `test_win_service_extra`, and `test_win_service_payload_limits` passed; CTest for those three tests passed.
+
 ## Validation
 
 Acceptance criteria evidence:
@@ -606,6 +630,7 @@ Acceptance criteria evidence:
 - Vendor script now copies the full vendored C include/source subtrees so split C files are not missed.
 - Netdata PR #22649 review and SonarCloud findings were verified against SDK source and addressed in the SDK before re-vendoring.
 - Plugin-ipc Go modules now match Netdata's Go version, `go 1.26.0`.
+- Netdata PR #22649 SonarCloud security hotspots for C string/path copying were verified and addressed in the SDK before re-vendoring.
 
 Tests or equivalent validation:
 
@@ -617,6 +642,7 @@ Tests or equivalent validation:
 - `cd src/go && go test -count=1 ./...`: passed after restoring the required protocol-level lookup dispatch guard.
 - Win11 temp-copy Rust raw-service validation: 22 tests passed.
 - Win11 temp-copy Go service/raw and transport/windows validation: passed.
+- Win11 temp-copy C service validation: `test_win_service`, `test_win_service_extra`, and `test_win_service_payload_limits` built and passed under MSYS.
 - `git diff --check`: passed.
 - `bash -n tests/run-windows-bench.sh tests/test_windows_bench_stability_policy.sh vendor-to-netdata.sh`: passed.
 - `codacy-analysis analyze --files ... --output-format json`: 0 issues; known local Revive adapter invocation error remains.
@@ -632,6 +658,7 @@ Reviewer findings:
 - GitHub AI findings for `netipc_uds_send.c` were manually verified and addressed.
 - GitHub review-thread findings from Netdata PR #22649 were manually verified and addressed where they were real.
 - SonarCloud PR findings for Netdata PR #22649 were queried and addressed in the SDK source before re-vendoring.
+- SonarCloud PR security hotspots for C `strncpy` and open-ended config field length/copy patterns were queried and addressed in the SDK source before re-vendoring.
 - No external AI reviewer was used for this increment.
 
 Same-failure scan:
@@ -641,6 +668,7 @@ Same-failure scan:
 - Same benchmark batch sizing nondeterminism pattern was found and fixed in POSIX and Windows Go benchmark drivers.
 - Same dead Go service-level lookup dispatch guard was found and removed in apps lookup and cgroups lookup.
 - Similar Go protocol-level lookup dispatch guards were tested and kept because regression coverage proves they still guard an overflow state.
+- Same C `strncpy` path-copy pattern was found beyond the reported UDS lifecycle file in POSIX SHM context path storage and fixed in the same increment.
 
 Sensitive data gate:
 

src/libnetdata/netipc/src/service/netipc_service_common.c

@@ -63,14 +63,20 @@ uint32_t nipc_service_common_typed_response_batch_items(uint32_t max_request_bat
     return max_request_batch_items;
 }
 
-static void copy_cstr_field(char *dst, size_t dst_size, const char *src)
+void nipc_service_common_copy_cstr_field(char *dst, size_t dst_size, const char *src)
 {
-    if (!src || dst_size == 0)
+    if (!dst || dst_size == 0)
         return;
 
-    size_t len = strlen(src);
-    if (len >= dst_size)
-        len = dst_size - 1;
+    dst[0] = '\0';
+    if (!src)
+        return;
+
+    size_t len = 0;
+    size_t max_copy = dst_size - 1;
+    while (len < max_copy && src[len] != '\0')
+        len++;
+
     memcpy(dst, src, len);
     dst[len] = '\0';
 }
@@ -83,8 +89,8 @@ void nipc_service_common_client_init(nipc_client_ctx_t *ctx,
     ctx->state = NIPC_CLIENT_DISCONNECTED;
     ctx->session_valid = false;
     ctx->shm = NULL;
-    copy_cstr_field(ctx->run_dir, sizeof(ctx->run_dir), run_dir);
-    copy_cstr_field(ctx->service_name, sizeof(ctx->service_name), service_name);
+    nipc_service_common_copy_cstr_field(ctx->run_dir, sizeof(ctx->run_dir), run_dir);
+    nipc_service_common_copy_cstr_field(ctx->service_name, sizeof(ctx->service_name), service_name);
 }
 
 void nipc_service_common_client_status(const nipc_client_ctx_t *ctx,

src/libnetdata/netipc/src/service/netipc_service_common.h

@@ -30,6 +30,7 @@ bool nipc_service_common_mul_would_overflow(size_t count, size_t size);
 uint32_t nipc_service_common_request_payload_default(void);
 uint32_t nipc_service_common_response_payload_default(void);
 uint32_t nipc_service_common_typed_response_batch_items(uint32_t max_request_batch_items);
+void nipc_service_common_copy_cstr_field(char *dst, size_t dst_size, const char *src);
 
 void nipc_service_common_client_init(nipc_client_ctx_t *ctx,
                                      const char *run_dir,

src/libnetdata/netipc/src/service/netipc_service_posix_server.c

@@ -117,21 +117,11 @@ nipc_error_t nipc_service_platform_server_init_raw(
     if (worker_count < 1)
         worker_count = 1;
 
-    /* Store config */
-    {
-        size_t len = strlen(run_dir);
-        if (len >= sizeof(server->run_dir))
-            len = sizeof(server->run_dir) - 1;
-        memcpy(server->run_dir, run_dir, len);
-        server->run_dir[len] = '\0';
-    }
-    {
-        size_t len = strlen(service_name);
-        if (len >= sizeof(server->service_name))
-            len = sizeof(server->service_name) - 1;
-        memcpy(server->service_name, service_name, len);
-        server->service_name[len] = '\0';
-    }
+    nipc_service_common_copy_cstr_field(server->run_dir, sizeof(server->run_dir),
+                                        run_dir);
+    nipc_service_common_copy_cstr_field(server->service_name,
+                                        sizeof(server->service_name),
+                                        service_name);
 
     server->handler = handler;
     server->handler_user = user;

src/libnetdata/netipc/src/service/netipc_service_win_server.c

@@ -196,21 +196,11 @@ nipc_error_t nipc_service_platform_server_init_raw(
     if (worker_count < 1)
         worker_count = 1;
 
-    /* Store config */
-    {
-        size_t len = strlen(run_dir);
-        if (len >= sizeof(server->run_dir))
-            len = sizeof(server->run_dir) - 1;
-        memcpy(server->run_dir, run_dir, len);
-        server->run_dir[len] = '\0';
-    }
-    {
-        size_t len = strlen(service_name);
-        if (len >= sizeof(server->service_name))
-            len = sizeof(server->service_name) - 1;
-        memcpy(server->service_name, service_name, len);
-        server->service_name[len] = '\0';
-    }
+    nipc_service_common_copy_cstr_field(server->run_dir, sizeof(server->run_dir),
+                                        run_dir);
+    nipc_service_common_copy_cstr_field(server->service_name,
+                                        sizeof(server->service_name),
+                                        service_name);
 
     server->handler = handler;
     server->handler_user = user;

src/libnetdata/netipc/src/transport/posix/netipc_shm.c

@@ -35,6 +35,21 @@ static inline uint32_t align64(uint32_t v)
     return (v + (NIPC_SHM_REGION_ALIGNMENT - 1)) & ~(uint32_t)(NIPC_SHM_REGION_ALIGNMENT - 1);
 }
 
+static int copy_cstr_checked(char *dst, size_t dst_size, const char *src)
+{
+    if (!dst || !src || dst_size == 0)
+        return -1;
+
+    size_t len = 0;
+    while (len < dst_size && src[len] != '\0')
+        len++;
+    if (len == dst_size)
+        return -1;
+
+    memcpy(dst, src, len + 1);
+    return 0;
+}
+
 /* Validate service_name: only [a-zA-Z0-9._-], non-empty, not "." or "..". */
 static int validate_service_name(const char *name)
 {
@@ -390,8 +405,15 @@ nipc_shm_error_t nipc_shm_server_create(const char *run_dir,
     out->local_resp_seq    = 0;
     out->spin_tries        = NIPC_SHM_DEFAULT_SPIN;
     out->owner_generation  = hdr->owner_generation;
-    strncpy(out->path, path, sizeof(out->path) - 1);
-    out->path[sizeof(out->path) - 1] = '\0';
+    if (copy_cstr_checked(out->path, sizeof(out->path), path) != 0) {
+        munmap(map, region_size);
+        close(fd);
+        unlinkat(dir_fd, shm_name, 0);
+        close(dir_fd);
+        memset(out, 0, sizeof(*out));
+        out->fd = -1;
+        return NIPC_SHM_ERR_PATH_TOO_LONG;
+    }
 
     close(dir_fd);
     return NIPC_SHM_OK;
@@ -570,8 +592,13 @@ nipc_shm_error_t nipc_shm_client_attach(const char *run_dir,
     out->local_resp_seq    = cur_resp_seq;
     out->spin_tries        = NIPC_SHM_DEFAULT_SPIN;
     out->owner_generation  = hdr->owner_generation;
-    strncpy(out->path, path, sizeof(out->path) - 1);
-    out->path[sizeof(out->path) - 1] = '\0';
+    if (copy_cstr_checked(out->path, sizeof(out->path), path) != 0) {
+        munmap(map, file_size);
+        close(fd);
+        memset(out, 0, sizeof(*out));
+        out->fd = -1;
+        return NIPC_SHM_ERR_PATH_TOO_LONG;
+    }
 
     return NIPC_SHM_OK;
 }

src/libnetdata/netipc/src/transport/posix/netipc_uds_lifecycle.c

@@ -12,6 +12,28 @@
 #include <sys/stat.h>
 #include <sys/un.h>
 
+static int copy_cstr_checked(char *dst, size_t dst_size, const char *src)
+{
+    if (!dst || !src || dst_size == 0)
+        return -1;
+
+    size_t len = 0;
+    while (len < dst_size && src[len] != '\0')
+        len++;
+    if (len == dst_size)
+        return -1;
+
+    memcpy(dst, src, len + 1);
+    return 0;
+}
+
+static int fill_sockaddr_path(struct sockaddr_un *addr, const char *path)
+{
+    memset(addr, 0, sizeof(*addr));
+    addr->sun_family = AF_UNIX;
+    return copy_cstr_checked(addr->sun_path, sizeof(addr->sun_path), path);
+}
+
 static int validate_service_name(const char *name)
 {
     if (!name || name[0] == '\0')
@@ -118,9 +140,10 @@ int nipc_uds_check_and_recover_stale(const char *run_dir,
         return -1;
 
     struct sockaddr_un addr;
-    memset(&addr, 0, sizeof(addr));
-    addr.sun_family = AF_UNIX;
-    strncpy(addr.sun_path, path, sizeof(addr.sun_path) - 1);
+    if (fill_sockaddr_path(&addr, path) != 0) {
+        close(probe);
+        return -1;
+    }
 
     int ret;
     if (connect(probe, (struct sockaddr *)&addr, sizeof(addr)) == 0) {
@@ -176,9 +199,10 @@ nipc_uds_error_t nipc_uds_listen(const char *run_dir,
         return NIPC_UDS_ERR_SOCKET;
 
     struct sockaddr_un addr;
-    memset(&addr, 0, sizeof(addr));
-    addr.sun_family = AF_UNIX;
-    strncpy(addr.sun_path, path, sizeof(addr.sun_path) - 1);
+    if (fill_sockaddr_path(&addr, path) != 0) {
+        close(fd);
+        return NIPC_UDS_ERR_PATH_TOO_LONG;
+    }
 
     if (bind(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
         close(fd);
@@ -194,8 +218,13 @@ nipc_uds_error_t nipc_uds_listen(const char *run_dir,
 
     out->fd = fd;
     out->config = *config;
-    strncpy(out->path, path, sizeof(out->path) - 1);
-    out->path[sizeof(out->path) - 1] = '\0';
+    if (copy_cstr_checked(out->path, sizeof(out->path), path) != 0) {
+        close(fd);
+        unlink(path);
+        memset(out, 0, sizeof(*out));
+        out->fd = -1;
+        return NIPC_UDS_ERR_PATH_TOO_LONG;
+    }
 
     return NIPC_UDS_OK;
 }
@@ -243,9 +272,10 @@ nipc_uds_error_t nipc_uds_connect(const char *run_dir,
         return NIPC_UDS_ERR_SOCKET;
 
     struct sockaddr_un addr;
-    memset(&addr, 0, sizeof(addr));
-    addr.sun_family = AF_UNIX;
-    strncpy(addr.sun_path, path, sizeof(addr.sun_path) - 1);
+    if (fill_sockaddr_path(&addr, path) != 0) {
+        close(fd);
+        return NIPC_UDS_ERR_PATH_TOO_LONG;
+    }
 
     if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
         close(fd);