Skip to content

Commit e8794b4

Browse files
jacob-ronstadtjacob-ronstadt
authored
Merge pull request #118 from microsoft/development
RI of development branch to main (02/15/24)
2 parents b8a5c9b + 4cf80ad commit e8794b4

69 files changed

Lines changed: 3112 additions & 2499 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/workflows/build-codeql.yaml‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ jobs:
2626
uses: actions/checkout@v2
2727
with:
2828
path: .
29-
29+
3030
- name: Download CodeQL CLI
3131
uses: i3h/download-release-asset@v1.2.0
3232
with:
@@ -42,14 +42,39 @@ jobs:
4242
shell: cmd
4343
continue-on-error: true # Required because robocopy returns 1 on success
4444
run: robocopy /S /move .\codeql-zip\codeql .\codeql-cli\
45-
45+
4646
- name: Install CodeQL pack dependencies
4747
shell: cmd
4848
run: |
4949
pushd .\src
5050
..\codeql-cli\codeql.cmd pack install
5151
popd
52-
52+
- name: codeql version test
53+
run: .\codeql-cli\codeql.exe version
54+
55+
- name: Setup Python
56+
uses: actions/setup-python@v2
57+
with:
58+
python-version: 3.11
59+
60+
- name: Install Python Packages
61+
run: |
62+
python -m pip install --upgrade pip
63+
pip install -r .\src\drivers\test\requirements.txt
64+
- name: Add msbuild to PATH
65+
uses: microsoft/setup-msbuild@v2
66+
67+
- name: Run test script
68+
shell: pwsh
69+
env:
70+
CONNECTION_STRING: ${{ secrets.CONNECTION_STRING }}
71+
ACCOUNT_KEY: ${{ secrets.ACCOUNT_KEY }}
72+
SHARE_NAME: ${{ secrets.SHARE_NAME }}
73+
CONTAINER_NAME: ${{ secrets.CONTAINER_NAME }}
74+
ACCOUNT_NAME: ${{ secrets.ACCOUNT_NAME }}
75+
76+
run: python src\drivers\test\build_create_analyze_test.py --codeql_path .\codeql-cli\codeql.exe --no_build --compare_results --connection_string "$env:CONNECTION_STRING" --share_name "$env:SHARE_NAME" --container_name "$env:CONTAINER_NAME" --storage_account_key "$env:ACCOUNT_KEY" --storage_account_name "$env:ACCOUNT_NAME"
77+
5378
- name: Build must-fix driver suite
5479
shell: cmd
5580
run: .\codeql-cli\codeql.cmd query compile --check-only windows_mustfix_partial.qls
@@ -65,3 +90,5 @@ jobs:
6590
- name: Build all Windows queries
6691
shell: cmd
6792
run: .\codeql-cli\codeql.cmd query compile --check-only .\src
93+
94+

‎.gitignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,6 @@ src/drivers/test/**/driver/x64/*
1111
src/drivers/test/TestDB/*
1212
src/drivers/test/working/*
1313
src/drivers/test/AnalysisFiles/*
14+
15+
#excel files
1416
**/*.xlsx

src/drivers/general/queries/DefaultPoolTag/DefaultPoolTag.qlhelp renamed to src/drivers/general/queries/DefaultPoolTag/DefaultPoolTag.qhelp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,22 @@
1111
</p>
1212
</recommendation>
1313
<example>
14-
<sample src="driver_snippet.c" />
14+
<p>
15+
In this example, the driver allocates memory with the default tag:
16+
</p>
17+
<sample language="c">
18+
PVOID InternalNonPagedAllocator(SIZE_T size) {
19+
return ExAllocatePool3(POOL_FLAG_NON_PAGED, size, ' mdW');
20+
}
21+
</sample>
22+
<p>
23+
The driver should use a custom tag instead:
24+
</p>
25+
<sample language="c">
26+
PVOID InternalNonPagedAllocator(SIZE_T size) {
27+
return ExAllocatePool3(POOL_FLAG_NON_PAGED, size, 'vdxE');
28+
}
29+
</sample>
1530
</example>
1631
<references>
1732
<li>

‎src/drivers/general/queries/ExaminedValue/ExaminedValue.qhelp‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
This warning indicates that the calling function is not checking the value of the specified variable, which was supplied by a function.
6+
</p>
7+
</overview>
8+
<recommendation>
9+
<p>
10+
Make sure to check the result of the function that is annotated with _Check_result_ or _Must_check_result.
11+
</p>
12+
</recommendation>
13+
<example>
14+
<p>
15+
In this example, the driver tries to acquire a mutex but does not check the return value. This can cause a concurrency bug.
16+
</p>
17+
<sample language="c"><![CDATA[
18+
KeTryToAcquireGuardedMutex(&sharedMutex);
19+
DoDriverWork();
20+
]]>
21+
</sample>
22+
<p>
23+
The driver should check if the mutex was successfully acquired before using it:
24+
</p>
25+
<sample language="c"><![CDATA[
26+
if(KeTryToAcquireGuardedMutex(&sharedMutex))
27+
{
28+
DoDriverWork();
29+
}
30+
else
31+
{
32+
// ...
33+
}
34+
]]>
35+
</sample>
36+
</example>
37+
<references>
38+
<li>
39+
<a href="https://docs.microsoft.com/en-us/cpp/code-quality/c28193">
40+
Warning C28193 | Microsoft Learn
41+
</a>
42+
</li>
43+
</references>
44+
<semmleNotes>
45+
<p>To reduce noise, this rule only reports violations if more than 75% of the other calls to this function have their return values checked.</p>
46+
<p>Note that this will still report issues if the value is only checked via ASSERTs that are compiled away at release time.</p>
47+
</semmleNotes>
48+
</qhelp>

src/drivers/wdm/queries/ExaminedValue/ExaminedValue.ql renamed to src/drivers/general/queries/ExaminedValue/ExaminedValue.ql

Lines changed: 79 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,75 +1,79 @@
1-
// Copyright (c) Microsoft Corporation.
2-
// Licensed under the MIT license.
3-
/**
4-
* @name Return value not examined (C28193)
5-
* @id cpp/drivers/examined-value
6-
* @kind problem
7-
* @description The returned value is annotated with the _Check_return_ or _Must_inspect_result_ annotation, but the calling function is either not using the value or is overwriting the value without examining it.
8-
* @platform Desktop
9-
* @security.severity Low
10-
* @feature.area Multiple
11-
* @impact Attack Surface Reduction
12-
* @repro.text The following code location calls a function annotated with _Check_return_ or _Must_inspect_result_ but does not check the returned value.
13-
* @owner.email sdat@microsoft.com
14-
* @opaqueid CQLD-C28134
15-
* @problem.severity warning
16-
* @precision high
17-
* @tags correctness
18-
* wddst
19-
* @scope general
20-
* @query-version v1
21-
*/
22-
23-
import cpp
24-
import drivers.libraries.SAL
25-
26-
/** A function that is annotated with either _Check_return_ or _Must_inspect_result_. */
27-
class ReturnMustBeCheckedFunction extends Function {
28-
SALCheckReturn scr;
29-
30-
ReturnMustBeCheckedFunction() { this.getADeclarationEntry() = scr.getDeclarationEntry() }
31-
}
32-
33-
/** A function call to a function annotated with either _Check_return_ or _Must_inspect_result_. */
34-
class ReturnMustBeCheckedFunctionCall extends FunctionCall {
35-
ReturnMustBeCheckedFunctionCall() { this.getTarget() instanceof ReturnMustBeCheckedFunction }
36-
}
37-
38-
/** Holds if an expression (a call to ReturnMustBeCheckedFunction in this case) is occuring in a void context. */
39-
predicate unUsed(Expr e) {
40-
e instanceof ExprInVoidContext
41-
or
42-
definition(_, e.getParent()) and
43-
not definitionUsePair(_, e.getParent(), _)
44-
}
45-
46-
/**
47-
* Returns true if a ReturnMustBeCheckedFunction has its return value checked more than 75% of the time.
48-
*/
49-
predicate callFrequency(ReturnMustBeCheckedFunction f, string message) {
50-
exists(Options opts, int used, int total, int percentage |
51-
used =
52-
count(ReturnMustBeCheckedFunctionCall fc |
53-
fc.getTarget() = f and not opts.okToIgnoreReturnValue(fc) and not unUsed(fc)
54-
) and
55-
total =
56-
count(ReturnMustBeCheckedFunctionCall fc |
57-
fc.getTarget() = f and not opts.okToIgnoreReturnValue(fc)
58-
) and
59-
used != total and
60-
percentage = used * 100 / total and
61-
percentage >= 75 and
62-
message =
63-
percentage.toString() +
64-
"% of calls to this function have their result checked. Checked return values = " +
65-
used.toString() + " total calls = " + total.toString()
66-
)
67-
}
68-
69-
from ReturnMustBeCheckedFunctionCall unused, string message
70-
where
71-
unUsed(unused) and
72-
not exists(Options opts | opts.okToIgnoreReturnValue(unused)) and
73-
callFrequency(unused.getTarget(), message) and
74-
not unused.getTarget().getName().matches("operator%") // exclude user defined operators
75-
select unused, "Result of call to " + unused.getTarget().getName() + " is ignored; " + message
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
/**
4+
* @name Return value not examined (C28193)
5+
* @id cpp/drivers/examined-value
6+
* @kind problem
7+
* @description The returned value is annotated with the _Check_return_ or _Must_inspect_result_ annotation, but the calling function is either not using the value or is overwriting the value without examining it.
8+
* @platform Desktop
9+
* @security.severity Low
10+
* @feature.area Multiple
11+
* @impact Attack Surface Reduction
12+
* @repro.text The following code location calls a function annotated with _Check_return_ or _Must_inspect_result_ but does not check the returned value.
13+
* @owner.email sdat@microsoft.com
14+
* @opaqueid CQLD-C28193
15+
* @problem.severity warning
16+
* @precision high
17+
* @tags correctness
18+
* wddst
19+
* @scope general
20+
* @query-version v1
21+
*/
22+
23+
import cpp
24+
import drivers.libraries.SAL
25+
26+
/** A function that is annotated with either _Check_return_ or _Must_inspect_result_. */
27+
class ReturnMustBeCheckedFunction extends Function {
28+
SALCheckReturn scr;
29+
30+
ReturnMustBeCheckedFunction() { this.getADeclarationEntry() = scr.getDeclarationEntry() }
31+
}
32+
33+
/** A function call to a function annotated with either _Check_return_ or _Must_inspect_result_. */
34+
class ReturnMustBeCheckedFunctionCall extends FunctionCall {
35+
ReturnMustBeCheckedFunctionCall() { this.getTarget() instanceof ReturnMustBeCheckedFunction }
36+
}
37+
38+
/** Holds if an expression (a call to ReturnMustBeCheckedFunction in this case) is occuring in a void context. */
39+
predicate unUsed(Expr e) {
40+
e instanceof ExprInVoidContext
41+
or
42+
definition(_, e.getParent()) and
43+
not definitionUsePair(_, e.getParent(), _)
44+
}
45+
46+
/**
47+
* Returns true if a ReturnMustBeCheckedFunction has its return value checked more than 75% of the time.
48+
*/
49+
predicate callFrequency(ReturnMustBeCheckedFunction f, string message) {
50+
exists(Options opts, int used, int total, int percentage |
51+
(
52+
used =
53+
count(ReturnMustBeCheckedFunctionCall fc |
54+
fc.getTarget() = f and not opts.okToIgnoreReturnValue(fc) and not unUsed(fc)
55+
) and
56+
total =
57+
count(ReturnMustBeCheckedFunctionCall fc |
58+
fc.getTarget() = f and not opts.okToIgnoreReturnValue(fc)
59+
)
60+
) and
61+
(
62+
used != total and
63+
percentage = used * 100 / total and
64+
percentage >= 75 and
65+
message =
66+
percentage.toString() +
67+
"% of calls to this function have their result checked. Checked return values = " +
68+
used.toString() + " total calls = " + total.toString()
69+
)
70+
)
71+
}
72+
73+
from ReturnMustBeCheckedFunctionCall unused, string message
74+
where
75+
unUsed(unused) and
76+
not exists(Options opts | opts.okToIgnoreReturnValue(unused)) and
77+
callFrequency(unused.getTarget(), message) and
78+
not unused.getTarget().getName().matches("operator%") // exclude user defined operators
79+
select unused, "Result of call to " + unused.getTarget().getName() + " is ignored; " + message

0 commit comments

Comments
 (0)