Skip to content

C++: Fix FP on cpp/cleartext-transmission#21857

Merged
MathiasVP merged 3 commits into
github:mainfrom
MathiasVP:fix-cleartext-fp
May 18, 2026
Merged

C++: Fix FP on cpp/cleartext-transmission#21857
MathiasVP merged 3 commits into
github:mainfrom
MathiasVP:fix-cleartext-fp

Conversation

@MathiasVP
Copy link
Copy Markdown
Contributor

@MathiasVP MathiasVP commented May 15, 2026

The query did not recognize that fscanf is potentially not a remote flow source because we didn't override the optional hasSocketInput predicate on RemoteFlowSourceFunction.

DCA shows lots of removed results in SAMATE. They're all instances of exactly the pattern I added in the query test: a fscanf call that writes to stdin.

@github-actions github-actions Bot added the C++ label May 15, 2026
@MathiasVP MathiasVP marked this pull request as ready for review May 16, 2026 11:11
@MathiasVP MathiasVP requested a review from a team as a code owner May 16, 2026 11:11
Copilot AI review requested due to automatic review settings May 16, 2026 11:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a false positive in the C++ cpp/cleartext-transmission query by refining the fscanf remote-source model so the analysis can distinguish socket-backed streams from obviously local streams (for example, stdin). It also adds a regression test and corresponding change notes for both the query and library packs.

Changes:

  • Refine the fscanf model by implementing RemoteFlowSourceFunction.hasSocketInput.
  • Add a regression test case ensuring fscanf(stdin, ...) is not treated as a remote source.
  • Add change notes documenting the query-result change and the underlying library model change.
Show a summary per file
File Description
cpp/ql/lib/semmle/code/cpp/models/implementations/Scanf.qll Updates the fscanf model to implement hasSocketInput.
cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/test3.cpp Adds a regression test for fscanf(stdin, ...) being treated as local.
cpp/ql/src/change-notes/2026-05-15-cleartext-transmission-fp.md Documents the cpp/cleartext-transmission false-positive reduction.
cpp/ql/lib/change-notes/2026-05-15-hasSocketInput-for-fscanf.md Documents the library model refinement for fscanf.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 2

Comment thread cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/test3.cpp
Comment thread cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/test3.cpp
Copy link
Copy Markdown
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FscanfModel / hasSocketInput model extension LGTM. 👍

It took me a few minutes to realize that cpp/cleartext-transmission actually looks for both transmission and receipt of cleartext sensitive data (fscanf would be receiving), which could be clearer.

@MathiasVP
Copy link
Copy Markdown
Contributor Author

It took me a few minutes to realize that cpp/cleartext-transmission actually looks for both transmission and receipt of cleartext sensitive data (fscanf would be receiving), which could be clearer.

Yeah, I agree. Thanks for the quick review!

@MathiasVP MathiasVP merged commit fcdce55 into github:main May 18, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants