fix: CFUSA-A006 false-positives from coincidental *, ++/--/+=/-= co-occurrence - #120
Merged
Merged
Conversation
…ccurrence
Root cause: a006_line() flagged ANY line containing a "++"/"--"/" += "/
" -= " token together with any "*" character anywhere on the line, with
no string-literal awareness and no requirement that the two relate to
the same variable. A CLI argv literal like
char *argv[] = {"cfusa", "--lcov", ...};
matched purely by coincidence: the "--" inside the "--lcov" string
literal, and the unrelated "*" in the pointer declaration.
Tightened to require both:
1. The operator occurrence is outside a string/char literal or a block
comment (previously unguarded at all -- sibling rule A007 already had
the string-literal guard via cfusa_match_outside_string(), A006 never
got it).
2. The identifier immediately touching the operator also appears
elsewhere on the line as "*<same identifier>" (a dereference, or a
pointer declaration of that name) -- so the arithmetic and the
pointer-ness plausibly apply to the SAME variable, not just two
unrelated tokens that happen to share a line (e.g. `count++` next to
an unrelated `*out` on the same line no longer fires).
Still a line-based heuristic (no real symbol table) -- not claiming a
false-positive-proof parser, just closing the specific coincidental-
co-occurrence class.
Result on this codebase: 545 -> 141 findings (74% reduction), spot-
checked a sample of the removed ones to confirm they were genuinely
unrelated tokens, not lost true positives. Existing true-positive tests
(ptr++, ptr += n) still fire; one previously-weak assertion
(test_a006_ptr_increment_fires only asserted "no crash", not that it
actually fired) is now a real TEST_ASSERT_TRUE, since the new logic
reliably detects it.
Also fixes an unrelated CFUSA-L004 (recursion-detector) false positive
this change's own implementation tripped: L004's file-wide brace
tracker doesn't count consecutive backslashes before a closing quote,
so a '\\' char literal in the new string/char-literal-skipping logic
desynchronized its brace count and misattributed later code in this
file to the wrong function. Worked around by spelling the backslash
comparison as a numeric constant instead of the '\\' literal (same
value, avoids the trigger) -- a pre-existing limitation in L004 itself,
out of scope to fix here.
4 new regression tests in tests/test_analyze_rules2.c: string-literal
co-occurrence, unrelated-identifier co-occurrence, and block-comment
co-occurrence, all confirmed silent; existing true-positive tests
strengthened/still passing.
Not a numbered issue -- fixed directly on request following a review
of GitHub Advanced Security bot comments on today's merged PRs.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Matt Jones <47545907+SoundMatt@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| while (s > line && (isalnum((unsigned char)s[-1]) || s[-1] == '_')) s--; | ||
| size_t n = (size_t)(e - s); | ||
| if (n == 0 || n >= out_sz) return 0; | ||
| memcpy(out, s, n); out[n] = '\0'; |
| while (isalnum((unsigned char)*s) || *s == '_') s++; | ||
| size_t n = (size_t)(s - b); | ||
| if (n == 0 || n >= out_sz) return 0; | ||
| memcpy(out, b, n); out[n] = '\0'; |
| * Used to confirm the identifier next to an arithmetic operator is | ||
| * plausibly a pointer, rather than an unrelated variable that merely | ||
| * shares the line with an unrelated '*'. */ | ||
| static int a006_has_star_ident(const char *line, const char *ident) |
| " (void)argv;\n" | ||
| "}\n", &rpt); | ||
| TEST_ASSERT_EQUAL(0, count_rule(&rpt, "CFUSA-A006")); | ||
| cfusa_report_free(&rpt); |
| " (void)count;\n" | ||
| "}\n", &rpt); | ||
| TEST_ASSERT_EQUAL(0, count_rule(&rpt, "CFUSA-A006")); | ||
| cfusa_report_free(&rpt); |
| { | ||
| size_t ilen = strlen(ident); | ||
| int in_str = 0, in_chr = 0, in_cmt = 0; | ||
| for (const char *p = line; *p; p++) { |
| if (strstr(line,"//")) return; /* preserves the prior line-comment exclusion */ | ||
|
|
||
| int in_str = 0, in_chr = 0, in_cmt = 0; | ||
| for (const char *q = line; *q; q++) { |
| " (void)argv;\n" | ||
| "}\n", &rpt); | ||
| TEST_ASSERT_EQUAL(0, count_rule(&rpt, "CFUSA-A006")); | ||
| cfusa_report_free(&rpt); |
| " (void)count;\n" | ||
| "}\n", &rpt); | ||
| TEST_ASSERT_EQUAL(0, count_rule(&rpt, "CFUSA-A006")); | ||
| cfusa_report_free(&rpt); |
| " (void)p; (void)mode;\n" | ||
| "}\n", &rpt); | ||
| TEST_ASSERT_EQUAL(0, count_rule(&rpt, "CFUSA-A006")); | ||
| cfusa_report_free(&rpt); |
| size_t ilen = strlen(ident); | ||
| int in_str = 0, in_chr = 0, in_cmt = 0; | ||
| for (const char *p = line; *p; p++) { | ||
| if (in_cmt) { if (p[0]=='*' && p[1]=='/') { in_cmt = 0; p++; } continue; } |
| if (in_cmt) { if (p[0]=='*' && p[1]=='/') { in_cmt = 0; p++; } continue; } | ||
| if (in_str) { if (*p == '"' && p[-1] != A006_BACKSLASH) in_str = 0; continue; } | ||
| if (in_chr) { if (*p == '\'' && p[-1] != A006_BACKSLASH) in_chr = 0; continue; } | ||
| if (p[0] == '/' && p[1] == '*') { in_cmt = 1; p++; continue; } |
|
|
||
| int in_str = 0, in_chr = 0, in_cmt = 0; | ||
| for (const char *q = line; *q; q++) { | ||
| if (in_cmt) { if (q[0]=='*' && q[1]=='/') { in_cmt = 0; q++; } continue; } |
| if (in_cmt) { if (q[0]=='*' && q[1]=='/') { in_cmt = 0; q++; } continue; } | ||
| if (in_str) { if (*q == '"' && q[-1] != A006_BACKSLASH) in_str = 0; continue; } | ||
| if (in_chr) { if (*q == '\'' && q[-1] != A006_BACKSLASH) in_chr = 0; continue; } | ||
| if (q[0] == '/' && q[1] == '*') { in_cmt = 1; q++; continue; } |
SoundMatt
added a commit
that referenced
this pull request
Aug 14, 2026
Covers three merged-but-unreleased PRs: #119 (unchecked fclose() fixes), #120 (CFUSA-A006 false-positive fix), #121 (CFUSA-L003 precision + ASIL-scaled severity). Also fixes two stale/inaccurate doc claims found while updating docs for this release: - docs/standards/misra-c.md and README.md both stated the check exit code reflects disposition acceptance. It doesn't -- cfusa check/lint never read .fusa-dispositions.json (issue #122). Corrected both to state this plainly instead of overclaiming. - README.md named the dispositions file .cfusa-dispositions.json -- that's the legacy fallback read path; the file cfusa disposition add actually writes is .fusa-dispositions.json. docs/standards/iso26262.md's "What scales by ASIL today" list gains an entry for CFUSA-L003's new ASIL-scaled severity (PR #121). Signed-off-by: Matt Jones <47545907+SoundMatt@users.noreply.github.com>
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
2 participants
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.
Fixes the
CFUSA-A006("pointer arithmetic") false-positive class found while reviewing GitHub Advanced Security bot comments on today's merged PRs — achar *argv[] = {"cfusa", "--lcov", ...}CLI-argv literal was flagged as pointer arithmetic purely by coincidence.Root cause
a006_line()flagged any line containing a++/--/+=/-=token together with any*character anywhere on the line — no string-literal awareness, and no requirement that the two relate to the same variable. The"--lcov"string literal's--and the unrelated*inchar *argv[]matched purely by coincidence. Sibling ruleA007already guards against string-literal matches viacfusa_match_outside_string();A006never got the same treatment.Fix
Tightened to require both:
*<same identifier>(a dereference, or a pointer declaration of that name) — the arithmetic and the pointer-ness must plausibly apply to the same variable, not just two unrelated tokens sharing a line.Still a line-based heuristic (no real symbol table) — not a false-positive-proof parser, just closes the specific coincidental-co-occurrence class.
Result
545 → 141 findings (74% reduction) across the codebase. Spot-checked a sample of the removed findings — all genuinely unrelated-token coincidences (e.g.
count++next to an unrelated*outdeclaration on the same line), not lost true positives. Existing true-positive tests (ptr++,ptr += n) still fire; one previously-weak assertion (test_a006_ptr_increment_firesonly asserted "no crash," not that it actually fired) is now a realTEST_ASSERT_TRUE, since the new logic reliably detects it.An unrelated bug found and fixed along the way
Implementing this tripped a pre-existing
CFUSA-L004(recursion-detector) false positive:L004's file-wide brace tracker doesn't count consecutive backslashes before a closing quote, so a'\\'char literal in my new string/char-literal-skipping logic desynchronized its brace count and misattributed later code in this file to the wrong function (a006_has_star_identreported as "recursive" when the actual call site was deep insidea006_line). Worked around by spelling the backslash comparison as a numeric constant ((char)0x5C) instead of the'\\'literal — same value, avoids the trigger. This is a pre-existing limitation inL004itself (already present incmd_lint.c's own similar code, just not triggered there), out of scope to fix in this PR.Testing
tests/test_analyze_rules2.c: string-literal co-occurrence, unrelated-identifier co-occurrence, and block-comment co-occurrence, all confirmed silent.ctest: 42/42 passing.cfusa check --dir .: 0 errors (confirmed theL004workaround resolves the self-check regression it would otherwise have introduced).cfusa trace --dir .:REQ-ANA006traced + tested, no dangling references.🤖 Generated with Claude Code