Skip to content

Commit 0a3ec7a

Browse files
committed
Resolve residual CodeQL findings
1 parent 8a23810 commit 0a3ec7a

4 files changed

Lines changed: 118 additions & 7 deletions

File tree

.agents/sow/done/SOW-0010-20260602-static-analysis-finding-cleanup.md

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,3 +709,88 @@ Artifact updates:
709709
- SOW lifecycle: this regression was appended after the prior SOW content; the
710710
SOW is marked `completed` and will be moved back to `done/` in the same
711711
commit as the implementation and docs.
712+
713+
## Regression - 2026-06-03 Residual Remote CodeQL Alerts
714+
715+
What broke:
716+
717+
- Commit `8a23810394997b0d6752c837d11a43343d85506b` fixed most restored
718+
GitHub Code Scanning findings, but the remote CodeQL run still reported nine
719+
open C alerts after SARIF ingestion: seven constant-comparison alerts and two
720+
TOCTOU stale-unlink alerts.
721+
722+
Evidence:
723+
724+
- GitHub Code Scanning reported the residual constant-comparison alerts in
725+
`src/libnetdata/netipc/src/protocol/netipc_protocol.c` and
726+
`src/libnetdata/netipc/src/service/netipc_service.c`; each was an overflow
727+
guard that is only reachable on 32-bit `size_t` builds because the affected
728+
counts are already capped by `uint32_t`.
729+
- GitHub Code Scanning reported the two residual TOCTOU alerts at the POSIX
730+
stale cleanup `unlink()` calls in
731+
`src/libnetdata/netipc/src/transport/posix/netipc_shm.c` and
732+
`src/libnetdata/netipc/src/transport/posix/netipc_uds.c`.
733+
- The stale cleanup race cannot be fully eliminated with POSIX path unlink
734+
semantics while preserving automatic stale socket/shared-memory cleanup. The
735+
implemented product constraint is that automatic stale unlink only runs in a
736+
process-owned run directory that is not group/world writable; the remaining
737+
CodeQL finding is intentionally narrow and source-documented.
738+
- The previous inline `// codeql[cpp/toctou-race-condition]` comments did not
739+
affect GitHub SARIF because the CodeQL configuration did not include the
740+
C/C++ `AlertSuppression.ql` query.
741+
742+
Why previous validation missed it:
743+
744+
- The local environment does not have the CodeQL CLI installed, so the only
745+
authoritative CodeQL validation point is the GitHub run after push.
746+
- The prior local validation proved the code built and local tools were clean,
747+
but it did not prove CodeQL SARIF suppression handling.
748+
749+
Repair plan:
750+
751+
- Keep the CodeQL rule enabled globally and add the C/C++ alert suppression
752+
query so the two reviewed stale-unlink suppressions are represented in SARIF.
753+
- Compile the remaining addition-overflow guards only on 32-bit `size_t`
754+
platforms, where they are real portability checks, so the 64-bit CodeQL build
755+
no longer sees constant-false comparisons.
756+
757+
Validation:
758+
759+
- `make` passed.
760+
- `actionlint` passed.
761+
- `git diff --check` passed.
762+
- `cmake -S . -B build-static -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXPORT_COMPILE_COMMANDS=ON`
763+
passed.
764+
- `cmake --build build-static --parallel --target netipc_protocol netipc_uds netipc_shm netipc_service`
765+
passed.
766+
- `clang-tidy -p build-static` on the C library sources passed with the
767+
repository's existing warning-only baseline.
768+
- The first attempted `cppcheck --project=build-static/compile_commands.json`
769+
validation failed because it scanned the whole compile database and surfaced
770+
existing test/benchmark findings unrelated to this SOW; that is not the
771+
workflow command shape.
772+
- The workflow-equivalent scoped cppcheck command passed:
773+
`cppcheck --enable=warning,performance,portability --error-exitcode=1 --force --inline-suppr --std=c11 --suppress=missingIncludeSystem --suppress=unmatchedSuppression -Isrc/libnetdata/netipc/include src/libnetdata/netipc`.
774+
- `flawfinder --minlevel=5 --error-level=5 src/libnetdata/netipc tests`
775+
passed with no level-5 findings.
776+
- `/usr/bin/ctest --test-dir build --output-on-failure` passed all 46 tests.
777+
- `osv-scanner scan --recursive --format sarif --output-file /tmp/plugin-ipc-osv-final.sarif .`
778+
exited 0, and the SARIF result count was 0.
779+
- `codacy-analysis analyze . --install-dependencies --output-format json --output /tmp/plugin-ipc-codacy-final.json --parallel-tools 2 --tool-timeout 900000`
780+
exited 0 with 0 issues and 0 tool errors.
781+
782+
Artifact updates:
783+
784+
- AGENTS.md: no update needed; project scanner workflow requirements remain
785+
accurate.
786+
- Runtime project skills: no update needed; the repository still has no runtime
787+
`project-*` skill.
788+
- Specs: no update needed; the stale cleanup behavior was already documented in
789+
the prior regression update.
790+
- End-user/operator docs: no update needed; this follow-up only makes CodeQL
791+
suppression handling explicit and preserves the already documented behavior.
792+
- End-user/operator skills: no update needed; the public integrator skill was
793+
already updated for the private runtime directory requirement.
794+
- SOW lifecycle: this residual remote CodeQL regression was appended after the
795+
prior SOW content; the SOW remains `completed` in `done/` with the follow-up
796+
implementation and validation recorded.

.github/codeql.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ queries:
66
- uses: security-extended
77
- uses: security-and-quality
88

9+
packs:
10+
c-cpp:
11+
- codeql/cpp-queries:AlertSuppression.ql
12+
913
query-filters:
1014
# Current baseline rule: CodeQL queries are enabled only when this branch has
1115
# no current alerts for them, or when the query is fixed in the same change.

src/libnetdata/netipc/src/protocol/netipc_protocol.c

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,8 +1137,10 @@ size_t nipc_cgroups_lookup_req_encode(const nipc_str_view_t *paths,
11371137
return 0;
11381138

11391139
size_t dir_size = (size_t)item_count * NIPC_LOOKUP_DIR_ENTRY_SIZE;
1140+
#if SIZE_MAX <= UINT32_MAX
11401141
if (dir_size > SIZE_MAX - NIPC_CGROUPS_LOOKUP_REQ_HDR_SIZE)
11411142
return 0;
1143+
#endif
11421144
size_t packed_start = NIPC_CGROUPS_LOOKUP_REQ_HDR_SIZE + dir_size;
11431145
if (buf_len < packed_start)
11441146
return 0;
@@ -1272,10 +1274,16 @@ size_t nipc_apps_lookup_req_encode(const uint32_t *pids,
12721274

12731275
size_t dir_size = (size_t)item_count * NIPC_LOOKUP_DIR_ENTRY_SIZE;
12741276
size_t key_size = (size_t)item_count * NIPC_APPS_LOOKUP_KEY_SIZE;
1277+
#if SIZE_MAX <= UINT32_MAX
12751278
if (dir_size > SIZE_MAX - NIPC_APPS_LOOKUP_REQ_HDR_SIZE)
12761279
return 0;
1280+
#endif
12771281
size_t packed_start = NIPC_APPS_LOOKUP_REQ_HDR_SIZE + dir_size;
1278-
if (key_size > SIZE_MAX - packed_start || buf_len < packed_start + key_size)
1282+
#if SIZE_MAX <= UINT32_MAX
1283+
if (key_size > SIZE_MAX - packed_start)
1284+
return 0;
1285+
#endif
1286+
if (buf_len < packed_start + key_size)
12791287
return 0;
12801288
if (item_count > 0 && !pids)
12811289
return 0;
@@ -1546,9 +1554,14 @@ void nipc_cgroups_lookup_builder_init(nipc_cgroups_lookup_builder_t *b,
15461554
b->data_offset = SIZE_MAX;
15471555
} else {
15481556
size_t dir_size = (size_t)max_items * NIPC_LOOKUP_DIR_ENTRY_SIZE;
1549-
b->data_offset = (dir_size > SIZE_MAX - NIPC_CGROUPS_LOOKUP_RESP_HDR_SIZE)
1550-
? SIZE_MAX
1551-
: NIPC_CGROUPS_LOOKUP_RESP_HDR_SIZE + dir_size;
1557+
#if SIZE_MAX <= UINT32_MAX
1558+
if (dir_size > SIZE_MAX - NIPC_CGROUPS_LOOKUP_RESP_HDR_SIZE) {
1559+
b->data_offset = SIZE_MAX;
1560+
} else
1561+
#endif
1562+
{
1563+
b->data_offset = NIPC_CGROUPS_LOOKUP_RESP_HDR_SIZE + dir_size;
1564+
}
15521565
}
15531566
}
15541567

@@ -1940,9 +1953,14 @@ void nipc_apps_lookup_builder_init(nipc_apps_lookup_builder_t *b,
19401953
b->data_offset = SIZE_MAX;
19411954
} else {
19421955
size_t dir_size = (size_t)max_items * NIPC_LOOKUP_DIR_ENTRY_SIZE;
1943-
b->data_offset = (dir_size > SIZE_MAX - NIPC_APPS_LOOKUP_RESP_HDR_SIZE)
1944-
? SIZE_MAX
1945-
: NIPC_APPS_LOOKUP_RESP_HDR_SIZE + dir_size;
1956+
#if SIZE_MAX <= UINT32_MAX
1957+
if (dir_size > SIZE_MAX - NIPC_APPS_LOOKUP_RESP_HDR_SIZE) {
1958+
b->data_offset = SIZE_MAX;
1959+
} else
1960+
#endif
1961+
{
1962+
b->data_offset = NIPC_APPS_LOOKUP_RESP_HDR_SIZE + dir_size;
1963+
}
19461964
}
19471965
}
19481966

src/libnetdata/netipc/src/service/netipc_service.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,11 +707,15 @@ static bool apps_lookup_request_size(uint32_t pid_count, size_t *size_out)
707707
return false;
708708
size_t dir = (size_t)pid_count * NIPC_LOOKUP_DIR_ENTRY_SIZE;
709709
size_t keys = (size_t)pid_count * NIPC_APPS_LOOKUP_KEY_SIZE;
710+
#if SIZE_MAX <= UINT32_MAX
710711
if (dir > SIZE_MAX - NIPC_APPS_LOOKUP_REQ_HDR_SIZE)
711712
return false;
713+
#endif
712714
size_t data = NIPC_APPS_LOOKUP_REQ_HDR_SIZE + dir;
715+
#if SIZE_MAX <= UINT32_MAX
713716
if (keys > SIZE_MAX - data)
714717
return false;
718+
#endif
715719
*size_out = data + keys;
716720
return true;
717721
}

0 commit comments

Comments
 (0)