Bring in additional rules from internal repo (both experimental and recommended.)#216
Open
NateD-MSFT wants to merge 3 commits intodevelopmentfrom
Open
Bring in additional rules from internal repo (both experimental and recommended.)#216NateD-MSFT wants to merge 3 commits intodevelopmentfrom
NateD-MSFT wants to merge 3 commits intodevelopmentfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds additional CodeQL rules (recommended + experimental) from an internal repository into the Windows driver suite, along with accompanying query help and examples.
Changes:
- Extend
src/windows-driver-suites/recommended.qlsto include new recommended queries. - Add new experimental security queries (taint/path/overflow/pointer-scaling) and shared QL libraries.
- Add query help (
.qhelp/.md) and example source files (.c/.cpp) for the added rules.
Reviewed changes
Copilot reviewed 60 out of 60 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/windows-driver-suites/recommended.qls | Adds newly imported recommended rules to the Windows driver “recommended” suite. |
| src/microsoft/experimental/Security/CWE-807/TaintedCondition.ql | New experimental taint query for untrusted conditions used in privilege-raising decisions. |
| src/microsoft/experimental/Security/CWE-807/TaintedCondition.qhelp | Help text for the CWE-807 experimental query. |
| src/microsoft/experimental/Security/CWE-807/TaintedCondition.md | Markdown documentation for the CWE-807 experimental query. |
| src/microsoft/experimental/Security/CWE-807/TaintedCondition.c | Example code referenced by the CWE-807 help/docs. |
| src/microsoft/experimental/Security/CWE-468/IncorrectPointerScalingCommon.qll | Shared utilities for CWE-468 pointer scaling queries. |
| src/microsoft/experimental/Security/CWE-468/IncorrectPointerScalingChar.ql | New experimental query detecting suspicious pointer scaling to char*. |
| src/microsoft/experimental/Security/CWE-468/IncorrectPointerScalingChar.qhelp | Help text for the CWE-468 char scaling query. |
| src/microsoft/experimental/Security/CWE-468/IncorrectPointerScalingChar.md | Markdown documentation for the CWE-468 char scaling query. |
| src/microsoft/experimental/Security/CWE-468/IncorrectPointerScalingChar.cpp | Example code referenced by CWE-468 docs. |
| src/microsoft/experimental/Security/CWE-190/TaintedAllocationSize.ql | New experimental taint query for user-controlled allocation sizes. |
| src/microsoft/experimental/Security/CWE-190/TaintedAllocationSize.qhelp | Help text for tainted allocation size. |
| src/microsoft/experimental/Security/CWE-190/TaintedAllocationSize.md | Markdown documentation for tainted allocation size. |
| src/microsoft/experimental/Security/CWE-190/TaintedAllocationSize.c | Example code for tainted allocation size. |
| src/microsoft/experimental/Security/CWE-190/IntegerOverflowTainted.ql | New experimental query for possible overflow in tainted arithmetic expressions. |
| src/microsoft/experimental/Security/CWE-190/IntegerOverflowTainted.qhelp | Help text for tainted integer overflow query. |
| src/microsoft/experimental/Security/CWE-190/IntegerOverflowTainted.md | Markdown documentation for tainted integer overflow query. |
| src/microsoft/experimental/Security/CWE-190/Bounded.qll | Adds a shared “bounded arithmetic” predicate used by multiple CWE-190 experimental queries. |
| src/microsoft/experimental/Security/CWE-190/ArithmeticTainted.ql | New experimental path query for user-controlled operands in arithmetic operations. |
| src/microsoft/experimental/Security/CWE-190/ArithmeticTainted.qhelp | Help text for arithmetic taint query. |
| src/microsoft/experimental/Security/CWE-190/ArithmeticTainted.md | Markdown documentation for arithmetic taint query. |
| src/microsoft/experimental/Security/CWE-190/ArithmeticTainted.c | Example code for arithmetic taint query. |
| src/microsoft/experimental/Security/CWE-022/examples/TaintedPathNormalize.c | Example showing path component validation for CWE-022. |
| src/microsoft/experimental/Security/CWE-022/examples/TaintedPathFolder.c | Example showing folder containment validation for CWE-022. |
| src/microsoft/experimental/Security/CWE-022/examples/TaintedPath.c | Example of vulnerable path construction for CWE-022. |
| src/microsoft/experimental/Security/CWE-022/TaintedPath.ql | New experimental taint query for file path injection/path traversal sinks. |
| src/microsoft/experimental/Security/CWE-022/TaintedPath.qhelp | Help text for experimental tainted path query. |
| src/microsoft/experimental/Security/CWE-022/TaintedPath.md | Markdown documentation for experimental tainted path query. |
| src/microsoft/Security/CWE/CWE-190/Bounded.qll | Adds shared “bounded arithmetic” predicate for public CWE-190 rules. |
| src/microsoft/Security/CWE/CWE-190/ArithmeticWithExtremeValues.ql | New recommended query: extreme values flowing into arithmetic ops with missing guards. |
| src/microsoft/Security/CWE/CWE-190/ArithmeticWithExtremeValues.qhelp | Help text for extreme-values arithmetic query. |
| src/microsoft/Security/CWE/CWE-190/ArithmeticWithExtremeValues.md | Markdown documentation for extreme-values arithmetic query. |
| src/microsoft/Security/CWE/CWE-190/ArithmeticWithExtremeValues.c | Example code for extreme-values arithmetic query. |
| src/microsoft/Security/CWE/CWE-190/ArithmeticUncontrolled.ql | New recommended query: uncontrolled (RNG) values flowing into risky arithmetic. |
| src/microsoft/Security/CWE/CWE-190/ArithmeticUncontrolled.qhelp | Help text for uncontrolled arithmetic query. |
| src/microsoft/Security/CWE/CWE-190/ArithmeticUncontrolled.md | Markdown documentation for uncontrolled arithmetic query. |
| src/microsoft/Security/CWE/CWE-190/ArithmeticUncontrolled.c | Example code for uncontrolled arithmetic query. |
| src/microsoft/Likely Bugs/Memory Management/UnsafeUseOfStrcat.ql | New recommended query detecting missing source-size checks before strcat. |
| src/microsoft/Likely Bugs/Memory Management/UnsafeUseOfStrcat.qhelp | Help text for unsafe strcat usage. |
| src/microsoft/Likely Bugs/Memory Management/UnsafeUseOfStrcat.md | Markdown documentation for unsafe strcat usage. |
| src/microsoft/Likely Bugs/Memory Management/UnsafeUseOfStrcat.cpp | Example code for unsafe strcat usage. |
| src/microsoft/Likely Bugs/Memory Management/StrncpyFlippedArgsGood.cpp | “Good” example for flipped strncpy size argument. |
| src/microsoft/Likely Bugs/Memory Management/StrncpyFlippedArgsBad.cpp | “Bad” example for flipped strncpy size argument. |
| src/microsoft/Likely Bugs/Memory Management/StrncpyFlippedArgs.ql | New recommended query detecting misuse of source buffer size as strncpy limit. |
| src/microsoft/Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp | Help text for flipped strncpy args query. |
| src/microsoft/Likely Bugs/Memory Management/StrncpyFlippedArgs.md | Markdown documentation for flipped strncpy args query. |
| src/microsoft/Likely Bugs/Memory Management/ImproperNullTerminationGood.cpp | “Good” example for null-termination before string function usage. |
| src/microsoft/Likely Bugs/Memory Management/ImproperNullTerminationBad.cpp | “Bad” example for improper/null termination before string function usage. |
| src/microsoft/Likely Bugs/Memory Management/ImproperNullTermination.ql | New recommended query detecting potentially unterminated strings reaching C string sinks. |
| src/microsoft/Likely Bugs/Memory Management/ImproperNullTermination.qhelp | Help text for improper null termination query. |
| src/microsoft/Likely Bugs/Memory Management/ImproperNullTermination.md | Markdown documentation for improper null termination query. |
| src/microsoft/Likely Bugs/Memory Management/Buffer.qll | Shared helper for buffer-size expressions used by multiple likely-bugs queries. |
| src/microsoft/Likely Bugs/Format/NonConstantFormat.ql | New recommended query: non-literal format strings flowing into printf-like APIs. |
| src/microsoft/Likely Bugs/Format/NonConstantFormat.qhelp | Help text for non-constant format string query. |
| src/microsoft/Likely Bugs/Format/NonConstantFormat.md | Markdown documentation for non-constant format string query. |
| src/microsoft/Likely Bugs/Format/NonConstantFormat-2-ok.c | Example: safer logging variant (constant format string to printf). |
| src/microsoft/Likely Bugs/Format/NonConstantFormat-2-good.c | Example: redesign to vprintf with varargs. |
| src/microsoft/Likely Bugs/Format/NonConstantFormat-2-bad.c | Example: vulnerable logging function passing non-constant format to printf. |
| src/microsoft/Likely Bugs/Format/NonConstantFormat-1-good.c | Example: safe echoing using "%s". |
| src/microsoft/Likely Bugs/Format/NonConstantFormat-1-bad.c | Example: vulnerable echoing using user-controlled format string. |
Comments suppressed due to low confidence (8)
src/microsoft/experimental/Security/CWE-468/IncorrectPointerScalingChar.ql:1
pointerArithmeticParentis defined as a function inIncorrectPointerScalingCommon.qll, but this query uses it as though it were a predicate (viaexists(pointerArithmeticParent(dest))) and also applies transitive closure (pointerArithmeticParent+) which only works on predicates. This is likely a compilation error. Consider changingpointerArithmeticParentinto a predicate likepredicate pointerArithmeticParent(Expr child, Expr parent)and then update these uses toexists(Expr p | pointerArithmeticParent(dest, p))andparent = pointerArithmeticParent+(dest)(or replace the closure with an explicit recursive predicate).
src/microsoft/experimental/Security/CWE-468/IncorrectPointerScalingChar.ql:1pointerArithmeticParentis defined as a function inIncorrectPointerScalingCommon.qll, but this query uses it as though it were a predicate (viaexists(pointerArithmeticParent(dest))) and also applies transitive closure (pointerArithmeticParent+) which only works on predicates. This is likely a compilation error. Consider changingpointerArithmeticParentinto a predicate likepredicate pointerArithmeticParent(Expr child, Expr parent)and then update these uses toexists(Expr p | pointerArithmeticParent(dest, p))andparent = pointerArithmeticParent+(dest)(or replace the closure with an explicit recursive predicate).
src/microsoft/experimental/Security/CWE-022/TaintedPath.ql:1Call.getTarget()returns a callable/function, not an expression, so callinggetUnspecifiedType()on it is invalid and will likely prevent the query from compiling. If the intent is to treat certain call results as barriers based on the call expression type, use the call expression's type (for example,node.asExpr().(Call).getUnspecifiedType()) or otherwise restructure the barrier condition around the correct AST node whose type you want to test.
src/microsoft/experimental/Security/CWE-190/ArithmeticTainted.ql:1- The helper
isSourceis typed toFS::FlowSource, butConfig.isSourcepasses aDataFlow::Node. Unless the QL type system can implicitly downcast here (typically it cannot), this will be a type error at compile time. A robust fix is to redefine the helper to acceptDataFlow::Nodeand explicitly cast inside (or implementConfig.isSourceassource instanceof FS::FlowSource). The same pattern appears in other new queries (for exampleIntegerOverflowTainted.ql,TaintedAllocationSize.ql,TaintedCondition.ql).
src/microsoft/experimental/Security/CWE-190/ArithmeticTainted.ql:1 - The helper
isSourceis typed toFS::FlowSource, butConfig.isSourcepasses aDataFlow::Node. Unless the QL type system can implicitly downcast here (typically it cannot), this will be a type error at compile time. A robust fix is to redefine the helper to acceptDataFlow::Nodeand explicitly cast inside (or implementConfig.isSourceassource instanceof FS::FlowSource). The same pattern appears in other new queries (for exampleIntegerOverflowTainted.ql,TaintedAllocationSize.ql,TaintedCondition.ql).
src/microsoft/experimental/Security/CWE-807/TaintedCondition.c:1 tHostis declared aschar*, sosizeof(tHost)is the size of the pointer (typically 4 or 8), not the string length. As an example in docs/help, this is misleading and could teach an incorrect pattern. Preferstrlen(tHost)(with appropriate null checks) or declaretHostaschar tHost[] = \"...\";sosizeof(tHost)reflects the array length.
src/microsoft/experimental/Security/CWE-022/examples/TaintedPath.c:1- This example won’t compile as a standalone C file: it uses
PATH_MAX,snprintf, andfopenwithout including headers that define them (andPATH_MAXis not guaranteed to be defined unless the appropriate headers are included). Consider adding the minimal includes (for example<stdio.h>and a header that definesPATH_MAXon your supported platforms, plus any others needed) so the sample is buildable and consistent with other CodeQL examples.
src/microsoft/experimental/Security/CWE-468/IncorrectPointerScalingCommon.qll:1 - Correct typo: 'ths' → 'this'.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/microsoft/Likely Bugs/Memory Management/ImproperNullTermination.ql
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: NateD-MSFT <34494373+NateD-MSFT@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
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.
This change adds the following queries from our internal CodeQL repo:
Recommended rules: these should be applicable to drivers as-is, but may have room for improvement; they have a low false positive rate in testing.
Experimental rules: these need additional work to be correctly applicable to drivers and currently have a high false positive rate. They are not part of our recommended set but are included to indicate future opportunities.