C++: exclude printf implementation internals from uncontrolled format string sinks#21493
C++: exclude printf implementation internals from uncontrolled format string sinks#21493MarkLee131 wants to merge 1 commit intogithub:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the C++ cpp/tainted-format-string query to reduce false positives that occur inside implementations of printf-like functions (and their wrappers), by excluding certain internal format-string sinks while preserving reporting at the outermost call sites.
Changes:
- Introduces a helper predicate to identify printf-like functions and wrapper functions forwarding format strings.
- Narrows the sink definition to exclude sinks occurring inside those identified functions.
You can also share your feedback on Copilot code review. Take the survey.
| * Holds if `f` is a printf-like function or a (possibly nested) wrapper | ||
| * that forwards a format-string parameter to one. | ||
| * | ||
| * Functions that *implement* printf-like behaviour (e.g. a custom |
There was a problem hiding this comment.
Copilot is right, we prefer American spellings in documentation, including in /** comments that may be extracted to documentation.
geoffw0
left a comment
There was a problem hiding this comment.
@MarkLee131 thank you for your contribution! I'm going to start some CI and probably try this out locally.
| not isPrintfImplementation(node.asExpr().getEnclosingFunction()) and | ||
| not isPrintfImplementation(node.asIndirectExpr().getEnclosingFunction()) |
There was a problem hiding this comment.
We can use the [] syntax from line 48 here as well:
| not isPrintfImplementation(node.asExpr().getEnclosingFunction()) and | |
| not isPrintfImplementation(node.asIndirectExpr().getEnclosingFunction()) | |
| not isPrintfImplementation([node.asExpr(), node.asIndirectExpr()].getEnclosingFunction()) |
| * Holds if `f` is a printf-like function or a (possibly nested) wrapper | ||
| * that forwards a format-string parameter to one. | ||
| * | ||
| * Functions that *implement* printf-like behaviour (e.g. a custom |
There was a problem hiding this comment.
Copilot is right, we prefer American spellings in documentation, including in /** comments that may be extracted to documentation.
| /** | ||
| * Holds if `f` is a printf-like function or a (possibly nested) wrapper | ||
| * that forwards a format-string parameter to one. | ||
| * | ||
| * Functions that *implement* printf-like behaviour (e.g. a custom | ||
| * `vsnprintf` variant) internally parse the caller-supplied format string | ||
| * and build small, bounded, local format strings such as `"%d"` or `"%ld"` | ||
| * for inner `sprintf` calls. Taint that reaches those inner calls via the | ||
| * parsed format specifier is not exploitable, so sinks inside such | ||
| * functions should be excluded. | ||
| */ |
Fix #21492
printf(tainted_str)smsg(tainted_fmt, ...)) remainflagged