Skip to content

C++: Add more registry flow sources and COM-related flow summaries - #22257

Merged
MathiasVP merged 9 commits into
github:mainfrom
MathiasVP:add-more-com-api-models
Jul 31, 2026
Merged

C++: Add more registry flow sources and COM-related flow summaries#22257
MathiasVP merged 9 commits into
github:mainfrom
MathiasVP:add-more-com-api-models

Conversation

@MathiasVP

Copy link
Copy Markdown
Contributor

This PR does two mostly unrelated changes in one go:

  1. First, we add two local flow sources that I forgot to add in C++: Add flow sources for winreg.h #22242
  2. Second, we add taint models for some common Component Object Model (COM) functions.

As part of (1) I also created a subclass of local flow sources called WindowsRegistrySource similar to what C# has. This shouldn't cause any semantic breakage since they're still local flow sources.

Commit-by-commit review recommended.

@MathiasVPMathiasVP added the no-change-note-required This PR does not need a change note label Jul 30, 2026
@MathiasVP
MathiasVPforce-pushed the add-more-com-api-models branch from 8634376 to cbb78caCompareJuly 30, 2026 15:05
@MathiasVP
MathiasVPforce-pushed the add-more-com-api-models branch from cbb78ca to 05c6fe3CompareJuly 30, 2026 15:10
@MathiasVP
MathiasVP marked this pull request as ready for review July 30, 2026 21:45
@MathiasVP
MathiasVP requested a review from a team as a code ownerJuly 30, 2026 21:45
CopilotAI review requested due to automatic review settings July 30, 2026 21:45

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

Registry value-name flows and wide-character coverage are missing, while the unconstrained StringFromGUID model risks incorrect summaries.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Adds Windows registry-specific flow sources and COM conversion summaries to C++ data-flow modeling.

Changes:

  • Introduces WindowsRegistrySource.
  • Models RegEnumValueA/W and COM conversion flows.
  • Expands related data-flow tests and expectations.
File summaries
FileDescription
cpp/ql/lib/ext/Windows.model.ymlAdds registry and COM models.
cpp/ql/lib/semmle/code/cpp/security/FlowSources.qllDefines the registry source abstraction.
cpp/ql/test/library-tests/dataflow/external-models/windows.cppAdds model test cases.
cpp/ql/test/library-tests/dataflow/external-models/steps.expectedRecords expected summary steps.
cpp/ql/test/library-tests/dataflow/external-models/sources.expectedRecords registry source classifications.
cpp/ql/test/library-tests/dataflow/external-models/flow.expectedRecords generated flow results.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 5
  • Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment threadcpp/ql/lib/ext/Windows.model.yml Outdated
Comment threadcpp/ql/lib/ext/Windows.model.yml Outdated
Comment threadcpp/ql/test/library-tests/dataflow/external-models/windows.cpp Outdated
Comment threadcpp/ql/lib/semmle/code/cpp/security/FlowSources.qll Outdated
Comment threadcpp/ql/lib/ext/Windows.model.yml Outdated
- ["", "", False, "CLSIDFromProgID", "", "", "Argument[*0]", "Argument[*1]", "taint", "manual"]
- ["", "", False, "CLSIDFromString", "", "", "Argument[*0]", "Argument[*1]", "taint", "manual"]
- ["", "", False, "StringFromCLSID", "", "", "Argument[*0]", "Argument[**1]", "taint", "manual"]
- ["", "", False, "StringFromGUID", "", "", "Argument[*0]", "Argument[*1]", "taint", "manual"]

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed in 9cf4d07. I have it on good authority that this function exists. But you're right when you say that it's not public. So let's leave it out!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Summary model is still here.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Doh. Good catch. Actually removed in a7c1978

@jketemajketema left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@MathiasVP
MathiasVP merged commit dc0be20 into github:mainJul 31, 2026
18 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++no-change-note-requiredThis PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@MathiasVP@jketema