Skip to content

Commit 310dc89

Browse files
rak3-shrak3-sh
authored
Merge branch 'main' into rp/a0-1-3-improvements
2 parents b150aed + e43fbbc commit 310dc89

File tree

55 files changed

+248
-152
lines changed

Some content is hidden

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

55 files changed

+248
-152
lines changed

‎c/cert/src/codeql-pack.lock.yml

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
lockVersion: 1.0.0
33
dependencies:
44
codeql/cpp-all:
5-
version: 0.4.6
5+
version: 0.6.1
66
codeql/ssa:
7+
version: 0.0.14
8+
codeql/tutorial:
79
version: 0.0.7
810
compiled: false

‎c/cert/src/codeql-suites/cert-default.qls

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@
66
- path-problem
77
- exclude:
88
tags contain:
9-
- external/cert/default-disabled
9+
- external/cert/default-disabled

‎c/cert/src/qlpack.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,4 @@ suites: codeql-suites
55
license: MIT
66
dependencies:
77
codeql/common-c-coding-standards: '*'
8-
codeql/cpp-all: 0.4.6
8+
codeql/cpp-all: 0.6.1

‎c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.ql

+15-48
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414
import cpp
1515
import codingstandards.c.cert
1616
import semmle.code.cpp.security.FunctionWithWrappers
17-
import semmle.code.cpp.security.Security
17+
import semmle.code.cpp.security.FlowSources
1818
import semmle.code.cpp.ir.IR
1919
import semmle.code.cpp.ir.dataflow.TaintTracking
20-
import DataFlow::PathGraph
20+
import TaintedPath::PathGraph
2121

2222
// Query TaintedPath.ql from the CodeQL standard library
2323
/**
@@ -46,22 +46,6 @@ class FileFunction extends FunctionWithWrappers {
4646
override predicate interestingArg(int arg) { arg = 0 }
4747
}
4848

49-
Expr asSourceExpr(DataFlow::Node node) {
50-
result = node.asConvertedExpr()
51-
or
52-
result = node.asDefiningArgument()
53-
}
54-
55-
Expr asSinkExpr(DataFlow::Node node) {
56-
result =
57-
node.asOperand()
58-
.(SideEffectOperand)
59-
.getUse()
60-
.(ReadSideEffectInstruction)
61-
.getArgumentDef()
62-
.getUnconvertedResultExpression()
63-
}
64-
6549
/**
6650
* Holds for a variable that has any kind of upper-bound check anywhere in the program.
6751
* This is biased towards being inclusive and being a coarse overapproximation because
@@ -85,20 +69,16 @@ predicate hasUpperBoundsCheck(Variable var) {
8569
)
8670
}
8771

88-
class TaintedPathConfiguration extends TaintTracking::Configuration {
89-
TaintedPathConfiguration() { this = "TaintedPathConfiguration" }
90-
91-
override predicate isSource(DataFlow::Node node) { isUserInput(asSourceExpr(node), _) }
72+
module TaintedPathConfiguration implements DataFlow::ConfigSig {
73+
predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
9274

93-
override predicate isSink(DataFlow::Node node) {
75+
predicate isSink(DataFlow::Node node) {
9476
exists(FileFunction fileFunction |
95-
fileFunction.outermostWrapperFunctionCall(asSinkExpr(node), _)
77+
fileFunction.outermostWrapperFunctionCall(node.asIndirectArgument(), _)
9678
)
9779
}
9880

99-
override predicate isSanitizerIn(DataFlow::Node node) { this.isSource(node) }
100-
101-
override predicate isSanitizer(DataFlow::Node node) {
81+
predicate isBarrier(DataFlow::Node node) {
10282
node.asExpr().(Call).getTarget().getUnspecifiedType() instanceof ArithmeticType
10383
or
10484
exists(LoadInstruction load, Variable checkedVar |
@@ -107,32 +87,19 @@ class TaintedPathConfiguration extends TaintTracking::Configuration {
10787
hasUpperBoundsCheck(checkedVar)
10888
)
10989
}
110-
111-
predicate hasFilteredFlowPath(DataFlow::PathNode source, DataFlow::PathNode sink) {
112-
this.hasFlowPath(source, sink) and
113-
// The use of `isUserInput` in `isSink` in combination with `asSourceExpr` causes
114-
// duplicate results. Filter these duplicates. The proper solution is to switch to
115-
// using `LocalFlowSource` and `RemoteFlowSource`, but this currently only supports
116-
// a subset of the cases supported by `isUserInput`.
117-
not exists(DataFlow::PathNode source2 |
118-
this.hasFlowPath(source2, sink) and
119-
asSourceExpr(source.getNode()) = asSourceExpr(source2.getNode())
120-
|
121-
not exists(source.getNode().asConvertedExpr()) and exists(source2.getNode().asConvertedExpr())
122-
)
123-
}
12490
}
12591

92+
module TaintedPath = TaintTracking::Make<TaintedPathConfiguration>;
93+
12694
from
127-
FileFunction fileFunction, Expr taintedArg, Expr taintSource, TaintedPathConfiguration cfg,
128-
DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode, string taintCause, string callChain
95+
FileFunction fileFunction, Expr taintedArg, FlowSource taintSource,
96+
TaintedPath::PathNode sourceNode, TaintedPath::PathNode sinkNode, string callChain
12997
where
13098
not isExcluded(taintedArg, IO3Package::doNotPerformFileOperationsOnDevicesQuery()) and
131-
taintedArg = asSinkExpr(sinkNode.getNode()) and
99+
taintedArg = sinkNode.getNode().asIndirectArgument() and
132100
fileFunction.outermostWrapperFunctionCall(taintedArg, callChain) and
133-
cfg.hasFilteredFlowPath(sourceNode, sinkNode) and
134-
taintSource = asSourceExpr(sourceNode.getNode()) and
135-
isUserInput(taintSource, taintCause)
101+
TaintedPath::hasFlowPath(sourceNode, sinkNode) and
102+
taintSource = sourceNode.getNode()
136103
select taintedArg, sourceNode, sinkNode,
137104
"This argument to a file access function is derived from $@ and then passed to " + callChain + ".",
138-
taintSource, "user input (" + taintCause + ")"
105+
taintSource, "user input (" + taintSource.getSourceType() + ")"

‎c/cert/test/codeql-pack.lock.yml

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
lockVersion: 1.0.0
33
dependencies:
44
codeql/cpp-all:
5-
version: 0.4.6
5+
version: 0.6.1
66
codeql/ssa:
7+
version: 0.0.14
8+
codeql/tutorial:
79
version: 0.0.7
810
compiled: false

‎c/cert/test/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.expected

+2-6
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,12 @@
11
edges
2-
| test.c:20:15:20:23 | file_name | test.c:21:8:21:16 | file_name indirection |
32
| test.c:20:15:20:23 | scanf output argument | test.c:21:8:21:16 | file_name indirection |
4-
| test.c:45:15:45:23 | file_name | test.c:46:29:46:37 | file_name indirection |
53
| test.c:45:15:45:23 | scanf output argument | test.c:46:29:46:37 | file_name indirection |
64
nodes
7-
| test.c:20:15:20:23 | file_name | semmle.label | file_name |
85
| test.c:20:15:20:23 | scanf output argument | semmle.label | scanf output argument |
96
| test.c:21:8:21:16 | file_name indirection | semmle.label | file_name indirection |
10-
| test.c:45:15:45:23 | file_name | semmle.label | file_name |
117
| test.c:45:15:45:23 | scanf output argument | semmle.label | scanf output argument |
128
| test.c:46:29:46:37 | file_name indirection | semmle.label | file_name indirection |
139
subpaths
1410
#select
15-
| test.c:21:8:21:16 | file_name | test.c:20:15:20:23 | file_name | test.c:21:8:21:16 | file_name indirection | This argument to a file access function is derived from $@ and then passed to func(file_name), which calls fopen((unnamed parameter 0)). | test.c:20:15:20:23 | file_name | user input (scanf) |
16-
| test.c:46:29:46:37 | file_name | test.c:45:15:45:23 | file_name | test.c:46:29:46:37 | file_name indirection | This argument to a file access function is derived from $@ and then passed to CreateFile(lpFileName). | test.c:45:15:45:23 | file_name | user input (scanf) |
11+
| test.c:21:8:21:16 | file_name | test.c:20:15:20:23 | scanf output argument | test.c:21:8:21:16 | file_name indirection | This argument to a file access function is derived from $@ and then passed to func(file_name), which calls fopen((unnamed parameter 0)). | test.c:20:15:20:23 | scanf output argument | user input (value read by scanf) |
12+
| test.c:46:29:46:37 | file_name | test.c:45:15:45:23 | scanf output argument | test.c:46:29:46:37 | file_name indirection | This argument to a file access function is derived from $@ and then passed to CreateFile(lpFileName). | test.c:45:15:45:23 | scanf output argument | user input (value read by scanf) |

‎c/common/src/codeql-pack.lock.yml

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
lockVersion: 1.0.0
33
dependencies:
44
codeql/cpp-all:
5-
version: 0.4.6
5+
version: 0.6.1
66
codeql/ssa:
7+
version: 0.0.14
8+
codeql/tutorial:
79
version: 0.0.7
810
compiled: false

‎c/common/src/codingstandards/c/OutOfBounds.qll

+4-2
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,8 @@ module OOB {
712712
}
713713

714714
private class DynamicAllocationSource extends PointerToObjectSource instanceof AllocationExpr,
715-
FunctionCall {
715+
FunctionCall
716+
{
716717
DynamicAllocationSource() {
717718
// exclude OperatorNewAllocationFunction to only deal with raw malloc-style calls,
718719
// which do not apply a multiple to the size of the allocation passed to them.
@@ -905,7 +906,8 @@ module OOB {
905906
override predicate isNotNullTerminated() { none() }
906907
}
907908

908-
private class PointerToObjectSourceOrSizeToBufferAccessFunctionConfig extends DataFlow::Configuration {
909+
private class PointerToObjectSourceOrSizeToBufferAccessFunctionConfig extends DataFlow::Configuration
910+
{
909911
PointerToObjectSourceOrSizeToBufferAccessFunctionConfig() {
910912
this = "PointerToObjectSourceOrSizeToBufferAccessFunctionConfig"
911913
}

‎c/common/src/qlpack.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ version: 2.22.0-dev
33
license: MIT
44
dependencies:
55
codeql/common-cpp-coding-standards: '*'
6-
codeql/cpp-all: 0.4.6
6+
codeql/cpp-all: 0.6.1

‎c/common/test/codeql-pack.lock.yml

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
lockVersion: 1.0.0
33
dependencies:
44
codeql/cpp-all:
5-
version: 0.4.6
5+
version: 0.6.1
66
codeql/ssa:
7+
version: 0.0.14
8+
codeql/tutorial:
79
version: 0.0.7
810
compiled: false

‎c/misra/src/codeql-pack.lock.yml

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
lockVersion: 1.0.0
33
dependencies:
44
codeql/cpp-all:
5-
version: 0.4.6
5+
version: 0.6.1
66
codeql/ssa:
7+
version: 0.0.14
8+
codeql/tutorial:
79
version: 0.0.7
810
compiled: false

‎c/misra/src/codeql-suites/misra-default.qls

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,4 @@
77
- exclude:
88
tags contain:
99
- external/misra/audit
10-
- external/misra/default-disabled
10+
- external/misra/default-disabled

‎c/misra/src/qlpack.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,4 @@ suites: codeql-suites
55
license: MIT
66
dependencies:
77
codeql/common-c-coding-standards: '*'
8-
codeql/cpp-all: 0.4.6
8+
codeql/cpp-all: 0.6.1

‎c/misra/test/codeql-pack.lock.yml

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
lockVersion: 1.0.0
33
dependencies:
44
codeql/cpp-all:
5-
version: 0.4.6
5+
version: 0.6.1
66
codeql/ssa:
7+
version: 0.0.14
8+
codeql/tutorial:
79
version: 0.0.7
810
compiled: false

‎change_notes/2022-03-16-update-for-dataflow-changes.md

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- `FIO32-C` - `DoNotPerformFileOperationsOnDevices.ql`:
2+
- The query was updated to work with the latest version of the dataflow library.

‎change_notes/2023-07-11-lambda-expr-without-param-list.md

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- `A5-1-3` - Only consider lambdas that have zero arguments, since any lambda with non-zero arguments will have an explicit argument list.

‎change_notes/2023-07-26-unused-local-variable.md

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- `M0-1-3` - Consider constexpr variables used in template instantiations as "used".

‎change_notes/2023-08-16-update-to-2.12.7.md

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Updated the supported CodeQL version to `2.12.7`.

‎change_notes/2023-08-30-a15-2-2-no-zero-paths.md

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- `A15-2-2` - all results now include an associated exception flow path to avoid a CodeQL CLI bug in 2.12.7. This includes results where an exception is thrown directly in the constructor.

‎cpp/autosar/src/codeql-pack.lock.yml

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
lockVersion: 1.0.0
33
dependencies:
44
codeql/cpp-all:
5-
version: 0.4.6
5+
version: 0.6.1
66
codeql/ssa:
7+
version: 0.0.14
8+
codeql/tutorial:
79
version: 0.0.7
810
compiled: false

‎cpp/autosar/src/codeql-suites/autosar-advisory.qls

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@
88
- external/autosar/obligation/advisory
99
- exclude:
1010
tags contain:
11-
- external/autosar/audit
11+
- external/autosar/audit

‎cpp/autosar/src/codeql-suites/autosar-audit.qls

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,4 @@
55
- problem
66
- path-problem
77
tags contain:
8-
- external/autosar/audit
8+
- external/autosar/audit

‎cpp/autosar/src/codeql-suites/autosar-default.qls

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,4 @@
77
- exclude:
88
tags contain:
99
- external/autosar/audit
10-
- external/autosar/default-disabled
10+
- external/autosar/default-disabled

‎cpp/autosar/src/codeql-suites/autosar-required.qls

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@
88
- external/autosar/obligation/required
99
- exclude:
1010
tags contain:
11-
- external/autosar/audit
11+
- external/autosar/audit

‎cpp/autosar/src/codeql-suites/autosar-single-translation-unit.qls

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@
99
- exclude:
1010
tags contain:
1111
- external/autosar/audit
12-
- external/autosar/default-disabled
12+
- external/autosar/default-disabled

‎cpp/autosar/src/qlpack.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,4 @@ suites: codeql-suites
55
license: MIT
66
dependencies:
77
codeql/common-cpp-coding-standards: '*'
8-
codeql/cpp-all: 0.4.6
8+
codeql/cpp-all: 0.6.1

‎cpp/autosar/src/rules/A15-2-2/ConstructorErrorLeavesObjectInInvalidState.ql

+29-13
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,13 @@ class NewWrapperFunction extends Function {
5353

5454
/** An expression on which `delete` is called, directly or indirectly. */
5555
class DeletedExpr extends Expr {
56+
pragma[noinline, nomagic]
5657
DeletedExpr() {
57-
this = any(DeleteExpr deleteExpr).getExpr() or
58+
this = any(DeleteExpr deleteExpr).getExpr()
59+
or
5860
exists(DeleteWrapperFunction dwf, FunctionCall call |
59-
this = call.getArgument(dwf.getADeleteParameter().getIndex())
61+
this = call.getArgument(dwf.getADeleteParameter().getIndex()) and
62+
call.getTarget() = dwf
6063
)
6164
}
6265
}
@@ -75,6 +78,14 @@ class DeleteWrapperFunction extends Function {
7578
Parameter getADeleteParameter() { result = p }
7679
}
7780

81+
class ExceptionThrowingConstructor extends ExceptionThrowingFunction, Constructor {
82+
ExceptionThrowingConstructor() {
83+
exists(getAFunctionThrownType(this, _)) and
84+
// The constructor is within the users source code
85+
exists(getFile().getRelativePath())
86+
}
87+
}
88+
7889
class ExceptionThrownInConstructor extends ExceptionThrowingExpr {
7990
Constructor c;
8091

@@ -87,24 +98,20 @@ class ExceptionThrownInConstructor extends ExceptionThrowingExpr {
8798
Constructor getConstructor() { result = c }
8899
}
89100

90-
/**
91-
* Add the `nodes` predicate to ensure results with an empty path are still reported.
92-
*/
93-
query predicate nodes(ExceptionFlowNode node) { any() }
94-
95101
from
96-
Constructor c, ExceptionThrownInConstructor throwingExpr, NewAllocationExpr newExpr,
97-
ExceptionFlowNode exceptionSource, ExceptionFlowNode functionNode
102+
ExceptionThrowingConstructor c, ExceptionThrownInConstructor throwingExpr,
103+
NewAllocationExpr newExpr, ExceptionFlowNode exceptionSource,
104+
ExceptionFlowNode throwingExprFlowNode, ExceptionFlowNode reportingNode
98105
where
99106
not isExcluded(c, Exceptions2Package::constructorErrorLeavesObjectInInvalidStateQuery()) and
100107
not isNoExceptTrue(c) and
101108
// Constructor must exit with an exception
102109
c = throwingExpr.getConstructor() and
103-
throwingExpr.hasExceptionFlowReflexive(exceptionSource, functionNode, _) and
110+
throwingExpr.hasExceptionFlowReflexive(exceptionSource, throwingExprFlowNode, _) and
104111
exists(ExceptionFlowNode mid |
105112
edges*(exceptionSource, mid) and
106113
newExpr.getASuccessor+() = mid.asThrowingExpr() and
107-
edges*(mid, functionNode) and
114+
edges*(mid, throwingExprFlowNode) and
108115
not exists(ExceptionFlowNode prior | edges(prior, mid) |
109116
prior.asCatchBlock().getEnclosingFunction() = c
110117
)
@@ -123,7 +130,16 @@ where
123130
DataFlow::localFlow(DataFlow::exprNode(newExpr), DataFlow::exprNode(deletedExpr)) and
124131
newExpr.getASuccessor+() = deletedExpr and
125132
deletedExpr.getASuccessor+() = throwingExpr
126-
)
127-
select c, exceptionSource, functionNode, "Constructor throws $@ and allocates memory at $@",
133+
) and
134+
// In CodeQL CLI 2.12.7 there is a bug which causes an infinite loop during results interpretation
135+
// when a result includes more than maxPaths paths and also includes a path with no edges i.e.
136+
// where the source and sink node are the same.
137+
// To avoid this edge case, if we report a path where the source and sink are the same (i.e the
138+
// throwingExpr directly throws an exception), we adjust the sink node to report the constructor,
139+
// which creates a one step path from the throwingExprFlowNode to the constructor node.
140+
if throwingExprFlowNode = exceptionSource
141+
then reportingNode.asFunction() = c and edges(throwingExprFlowNode, reportingNode)
142+
else reportingNode = throwingExprFlowNode
143+
select c, exceptionSource, reportingNode, "Constructor throws $@ and allocates memory at $@",
128144
throwingExpr, throwingExpr.(ThrowingExpr).getAnExceptionType().getExceptionName(), newExpr,
129145
"alloc"

‎cpp/autosar/src/rules/A5-1-3/LambdaExpressionWithoutParameterList.ql

+4
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ where
2121
not isExcluded(lambda, LambdasPackage::lambdaExpressionWithoutParameterListQuery()) and
2222
lambdaFunction = lambda.getLambdaFunction() and
2323
not lambdaFunction.isAffectedByMacro() and
24+
// If it has a parameter, then it will have an
25+
// explicit parameter list. Therefore, proceed to check only if the lambda
26+
// does not have any parameters.
27+
not exists (lambdaFunction.getAParameter()) and
2428
// The extractor doesn't store the syntactic information whether the parameter list
2529
// is enclosed in parenthesis. Therefore we cannot determine if the parameter list is
2630
// explicitly specified when the parameter list is empty.

0 commit comments

Comments
 (0)