C++: Add more Win32 flow sources#19591
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds Win32-specific dataflow tests and models for file reading and memory mapping APIs.
- Extends
windows.cpptest suite with ReadFile, ReadFileEx, NtReadFile, and various MapViewOfFile calls. - Updates
sources.expectedandflow.expectedto include flows from the new APIs. - Adds new entries in
Windows.model.ymland documents the feature in change notes.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cpp/ql/test/library-tests/dataflow/external-models/windows.cpp | Added tests for ReadFile, ReadFileEx, NtReadFile, and MapViewOfFile* |
| cpp/ql/test/library-tests/dataflow/external-models/sources.expected | Added expected source entries for the new Win32 APIs |
| cpp/ql/test/library-tests/dataflow/external-models/flow.expected | Updated provenance entries for the added test flows |
| cpp/ql/lib/ext/Windows.model.yml | Defined new flow source models for ReadFile, memory mapping, and NtReadFile |
| cpp/ql/lib/change-notes/2025-05-27-windows-sources-2.md | Documented the addition of the new flow source models |
Comments suppressed due to low confidence (1)
cpp/ql/test/library-tests/dataflow/external-models/windows.cpp:158
- [nitpick] The variable name 'p' is ambiguous; a more descriptive name like 'bufferPtr' would improve readability.
char* p = reinterpret_cast<char*>(overlapped.hEvent);
jketema
left a comment
jketema
left a comment
There was a problem hiding this comment.
LGTM is DCA is happy. Just some minor nits about documenting where the functions are coming from.
| using HANDLE = void*; | ||
| using DWORD = unsigned long; | ||
| using LPVOID = void*; | ||
| using LPDWORD = unsigned long*; | ||
| using PVOID = void*; | ||
| using ULONG_PTR = unsigned long*; | ||
| using SIZE_T = decltype(sizeof(0)); |
There was a problem hiding this comment.
We should probably also use these higher up in this file. We can do that as a follow-up.
Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com>
Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com>
Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com>
| char* p = reinterpret_cast<char*>(overlapped.hEvent); | ||
| sink(p); | ||
| sink(*p); // $ MISSING: ir |
There was a problem hiding this comment.
Is this still relevant with the hEvent model removed?
There was a problem hiding this comment.
I guess there's still flow at runtime, right? buffer and overlapped.hEvent alias, so the call to ReadFileEx will also write user-controlled data to overlapped.hEvent. So if we had proper alias support in dataflow (which we don't) we should get this flow.
|
DCA was uneventful. Merging! |
2 participants
This PR adds Win32 specific flow sources related to reading/memory mapping files.