Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update CodeQL CLI dependency to 2.12.7. #343

Merged
merged 24 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f1d5cc0
Merge branch 'main' into next
jketema Mar 1, 2023
05255c3
Fix XML deprecation warnings
jketema Mar 1, 2023
dd39c94
Update MISRA RULE-8-4 test for changes in latest CodeQL
jketema Mar 1, 2023
d75bda6
Fix `NULLMacro` deprecation warning
jketema Mar 1, 2023
4a64e8b
Fix test file formatting
jketema Mar 1, 2023
35c9fd2
Merge branch 'main' into next
jketema Mar 1, 2023
e1fb019
Merge branch 'main' into next
jketema Mar 6, 2023
5d7f642
Merge branch 'main' into next
jketema Mar 6, 2023
760c05b
Merge branch 'main' into next
jketema Mar 9, 2023
c2e7fa0
Update FIO32-C with the latest version of the query from CodeQL
jketema Mar 16, 2023
d1c0a5d
Add change note
jketema Mar 16, 2023
64ce6b4
Merge pull request #256 from jketema/tainted-path-update
jketema Mar 16, 2023
b45c846
Update to CodeQL CLI 2.12.7.
lcartey Aug 16, 2023
a810682
Add quick & dirty script for updating codeql dependencies
lcartey Aug 16, 2023
208b8f9
Update CodeQL dependencies for 2.12.7
lcartey Aug 16, 2023
2b7afbe
Merge commit '64ce6b45f3f7f143c2fdd4d3cfdbb8373cf74b35' into lcartey/…
lcartey Aug 16, 2023
91c5a92
Update test output for 2.12.7.
lcartey Aug 16, 2023
ea0d11f
Merge branch 'main' into lcartey/update-to-2.12
lcartey Aug 16, 2023
14cb4ff
Add change note.
lcartey Aug 16, 2023
2d0a6a6
Format the QL files
lcartey Aug 16, 2023
8eb193a
Update generate_modules qlpack, and update script.
lcartey Aug 16, 2023
1cc7e8b
Format OutOfBounds.qll
lcartey Aug 17, 2023
afc027b
A15-2-2: Address cartesian product
lcartey Aug 20, 2023
da1d12e
A15-2-2: Avoid infinite interpretation edge case
lcartey Aug 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion c/cert/src/codeql-pack.lock.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
lockVersion: 1.0.0
dependencies:
codeql/cpp-all:
version: 0.4.6
version: 0.6.1
codeql/ssa:
version: 0.0.14
codeql/tutorial:
version: 0.0.7
compiled: false
2 changes: 1 addition & 1 deletion c/cert/src/codeql-suites/cert-default.qls
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
- path-problem
- exclude:
tags contain:
- external/cert/default-disabled
- external/cert/default-disabled
2 changes: 1 addition & 1 deletion c/cert/src/qlpack.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ suites: codeql-suites
license: MIT
dependencies:
codeql/common-c-coding-standards: '*'
codeql/cpp-all: 0.4.6
codeql/cpp-all: 0.6.1
63 changes: 15 additions & 48 deletions c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.ql
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
import cpp
import codingstandards.c.cert
import semmle.code.cpp.security.FunctionWithWrappers
import semmle.code.cpp.security.Security
import semmle.code.cpp.security.FlowSources
import semmle.code.cpp.ir.IR
import semmle.code.cpp.ir.dataflow.TaintTracking
import DataFlow::PathGraph
import TaintedPath::PathGraph

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

Expr asSourceExpr(DataFlow::Node node) {
result = node.asConvertedExpr()
or
result = node.asDefiningArgument()
}

Expr asSinkExpr(DataFlow::Node node) {
result =
node.asOperand()
.(SideEffectOperand)
.getUse()
.(ReadSideEffectInstruction)
.getArgumentDef()
.getUnconvertedResultExpression()
}

/**
* Holds for a variable that has any kind of upper-bound check anywhere in the program.
* This is biased towards being inclusive and being a coarse overapproximation because
Expand All @@ -85,20 +69,16 @@ predicate hasUpperBoundsCheck(Variable var) {
)
}

class TaintedPathConfiguration extends TaintTracking::Configuration {
TaintedPathConfiguration() { this = "TaintedPathConfiguration" }

override predicate isSource(DataFlow::Node node) { isUserInput(asSourceExpr(node), _) }
module TaintedPathConfiguration implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node node) { node instanceof FlowSource }

override predicate isSink(DataFlow::Node node) {
predicate isSink(DataFlow::Node node) {
exists(FileFunction fileFunction |
fileFunction.outermostWrapperFunctionCall(asSinkExpr(node), _)
fileFunction.outermostWrapperFunctionCall(node.asIndirectArgument(), _)
)
}

override predicate isSanitizerIn(DataFlow::Node node) { this.isSource(node) }

override predicate isSanitizer(DataFlow::Node node) {
predicate isBarrier(DataFlow::Node node) {
node.asExpr().(Call).getTarget().getUnspecifiedType() instanceof ArithmeticType
or
exists(LoadInstruction load, Variable checkedVar |
Expand All @@ -107,32 +87,19 @@ class TaintedPathConfiguration extends TaintTracking::Configuration {
hasUpperBoundsCheck(checkedVar)
)
}

predicate hasFilteredFlowPath(DataFlow::PathNode source, DataFlow::PathNode sink) {
this.hasFlowPath(source, sink) and
// The use of `isUserInput` in `isSink` in combination with `asSourceExpr` causes
// duplicate results. Filter these duplicates. The proper solution is to switch to
// using `LocalFlowSource` and `RemoteFlowSource`, but this currently only supports
// a subset of the cases supported by `isUserInput`.
not exists(DataFlow::PathNode source2 |
this.hasFlowPath(source2, sink) and
asSourceExpr(source.getNode()) = asSourceExpr(source2.getNode())
|
not exists(source.getNode().asConvertedExpr()) and exists(source2.getNode().asConvertedExpr())
)
}
}

module TaintedPath = TaintTracking::Make<TaintedPathConfiguration>;

from
FileFunction fileFunction, Expr taintedArg, Expr taintSource, TaintedPathConfiguration cfg,
DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode, string taintCause, string callChain
FileFunction fileFunction, Expr taintedArg, FlowSource taintSource,
TaintedPath::PathNode sourceNode, TaintedPath::PathNode sinkNode, string callChain
where
not isExcluded(taintedArg, IO3Package::doNotPerformFileOperationsOnDevicesQuery()) and
taintedArg = asSinkExpr(sinkNode.getNode()) and
taintedArg = sinkNode.getNode().asIndirectArgument() and
fileFunction.outermostWrapperFunctionCall(taintedArg, callChain) and
cfg.hasFilteredFlowPath(sourceNode, sinkNode) and
taintSource = asSourceExpr(sourceNode.getNode()) and
isUserInput(taintSource, taintCause)
TaintedPath::hasFlowPath(sourceNode, sinkNode) and
taintSource = sourceNode.getNode()
select taintedArg, sourceNode, sinkNode,
"This argument to a file access function is derived from $@ and then passed to " + callChain + ".",
taintSource, "user input (" + taintCause + ")"
taintSource, "user input (" + taintSource.getSourceType() + ")"
4 changes: 3 additions & 1 deletion c/cert/test/codeql-pack.lock.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
lockVersion: 1.0.0
dependencies:
codeql/cpp-all:
version: 0.4.6
version: 0.6.1
codeql/ssa:
version: 0.0.14
codeql/tutorial:
version: 0.0.7
compiled: false
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
edges
| test.c:20:15:20:23 | file_name | test.c:21:8:21:16 | file_name indirection |
| test.c:20:15:20:23 | scanf output argument | test.c:21:8:21:16 | file_name indirection |
| test.c:45:15:45:23 | file_name | test.c:46:29:46:37 | file_name indirection |
| test.c:45:15:45:23 | scanf output argument | test.c:46:29:46:37 | file_name indirection |
nodes
| test.c:20:15:20:23 | file_name | semmle.label | file_name |
| test.c:20:15:20:23 | scanf output argument | semmle.label | scanf output argument |
| test.c:21:8:21:16 | file_name indirection | semmle.label | file_name indirection |
| test.c:45:15:45:23 | file_name | semmle.label | file_name |
| test.c:45:15:45:23 | scanf output argument | semmle.label | scanf output argument |
| test.c:46:29:46:37 | file_name indirection | semmle.label | file_name indirection |
subpaths
#select
| 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) |
| 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) |
| 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) |
| 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) |
4 changes: 3 additions & 1 deletion c/common/src/codeql-pack.lock.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
lockVersion: 1.0.0
dependencies:
codeql/cpp-all:
version: 0.4.6
version: 0.6.1
codeql/ssa:
version: 0.0.14
codeql/tutorial:
version: 0.0.7
compiled: false
6 changes: 4 additions & 2 deletions c/common/src/codingstandards/c/OutOfBounds.qll
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,8 @@ module OOB {
}

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

private class PointerToObjectSourceOrSizeToBufferAccessFunctionConfig extends DataFlow::Configuration {
private class PointerToObjectSourceOrSizeToBufferAccessFunctionConfig extends DataFlow::Configuration
{
PointerToObjectSourceOrSizeToBufferAccessFunctionConfig() {
this = "PointerToObjectSourceOrSizeToBufferAccessFunctionConfig"
}
Expand Down
2 changes: 1 addition & 1 deletion c/common/src/qlpack.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ version: 2.22.0-dev
license: MIT
dependencies:
codeql/common-cpp-coding-standards: '*'
codeql/cpp-all: 0.4.6
codeql/cpp-all: 0.6.1
4 changes: 3 additions & 1 deletion c/common/test/codeql-pack.lock.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
lockVersion: 1.0.0
dependencies:
codeql/cpp-all:
version: 0.4.6
version: 0.6.1
codeql/ssa:
version: 0.0.14
codeql/tutorial:
version: 0.0.7
compiled: false
4 changes: 3 additions & 1 deletion c/misra/src/codeql-pack.lock.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
lockVersion: 1.0.0
dependencies:
codeql/cpp-all:
version: 0.4.6
version: 0.6.1
codeql/ssa:
version: 0.0.14
codeql/tutorial:
version: 0.0.7
compiled: false
2 changes: 1 addition & 1 deletion c/misra/src/codeql-suites/misra-default.qls
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@
- exclude:
tags contain:
- external/misra/audit
- external/misra/default-disabled
- external/misra/default-disabled
2 changes: 1 addition & 1 deletion c/misra/src/qlpack.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ suites: codeql-suites
license: MIT
dependencies:
codeql/common-c-coding-standards: '*'
codeql/cpp-all: 0.4.6
codeql/cpp-all: 0.6.1
4 changes: 3 additions & 1 deletion c/misra/test/codeql-pack.lock.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
lockVersion: 1.0.0
dependencies:
codeql/cpp-all:
version: 0.4.6
version: 0.6.1
codeql/ssa:
version: 0.0.14
codeql/tutorial:
version: 0.0.7
compiled: false
2 changes: 2 additions & 0 deletions change_notes/2022-03-16-update-for-dataflow-changes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `FIO32-C` - `DoNotPerformFileOperationsOnDevices.ql`:
- The query was updated to work with the latest version of the dataflow library.
1 change: 1 addition & 0 deletions change_notes/2023-08-16-update-to-2.12.7.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Updated the supported CodeQL version to `2.12.7`.
1 change: 1 addition & 0 deletions change_notes/2023-08-30-a15-2-2-no-zero-paths.md
Original file line number Diff line number Diff line change
@@ -0,0 +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.
4 changes: 3 additions & 1 deletion cpp/autosar/src/codeql-pack.lock.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
lockVersion: 1.0.0
dependencies:
codeql/cpp-all:
version: 0.4.6
version: 0.6.1
codeql/ssa:
version: 0.0.14
codeql/tutorial:
version: 0.0.7
compiled: false
2 changes: 1 addition & 1 deletion cpp/autosar/src/codeql-suites/autosar-advisory.qls
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@
- external/autosar/obligation/advisory
- exclude:
tags contain:
- external/autosar/audit
- external/autosar/audit
2 changes: 1 addition & 1 deletion cpp/autosar/src/codeql-suites/autosar-audit.qls
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
- problem
- path-problem
tags contain:
- external/autosar/audit
- external/autosar/audit
2 changes: 1 addition & 1 deletion cpp/autosar/src/codeql-suites/autosar-default.qls
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@
- exclude:
tags contain:
- external/autosar/audit
- external/autosar/default-disabled
- external/autosar/default-disabled
2 changes: 1 addition & 1 deletion cpp/autosar/src/codeql-suites/autosar-required.qls
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@
- external/autosar/obligation/required
- exclude:
tags contain:
- external/autosar/audit
- external/autosar/audit
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@
- exclude:
tags contain:
- external/autosar/audit
- external/autosar/default-disabled
- external/autosar/default-disabled
2 changes: 1 addition & 1 deletion cpp/autosar/src/qlpack.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ suites: codeql-suites
license: MIT
dependencies:
codeql/common-cpp-coding-standards: '*'
codeql/cpp-all: 0.4.6
codeql/cpp-all: 0.6.1
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,13 @@ class NewWrapperFunction extends Function {

/** An expression on which `delete` is called, directly or indirectly. */
class DeletedExpr extends Expr {
pragma[noinline, nomagic]
DeletedExpr() {
this = any(DeleteExpr deleteExpr).getExpr() or
this = any(DeleteExpr deleteExpr).getExpr()
or
exists(DeleteWrapperFunction dwf, FunctionCall call |
this = call.getArgument(dwf.getADeleteParameter().getIndex())
this = call.getArgument(dwf.getADeleteParameter().getIndex()) and
call.getTarget() = dwf
)
}
}
Expand All @@ -75,6 +78,14 @@ class DeleteWrapperFunction extends Function {
Parameter getADeleteParameter() { result = p }
}

class ExceptionThrowingConstructor extends ExceptionThrowingFunction, Constructor {
ExceptionThrowingConstructor() {
exists(getAFunctionThrownType(this, _)) and
// The constructor is within the users source code
exists(getFile().getRelativePath())
}
}

class ExceptionThrownInConstructor extends ExceptionThrowingExpr {
Constructor c;

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

/**
* Add the `nodes` predicate to ensure results with an empty path are still reported.
*/
query predicate nodes(ExceptionFlowNode node) { any() }

from
Constructor c, ExceptionThrownInConstructor throwingExpr, NewAllocationExpr newExpr,
ExceptionFlowNode exceptionSource, ExceptionFlowNode functionNode
ExceptionThrowingConstructor c, ExceptionThrownInConstructor throwingExpr,
NewAllocationExpr newExpr, ExceptionFlowNode exceptionSource,
ExceptionFlowNode throwingExprFlowNode, ExceptionFlowNode reportingNode
where
not isExcluded(c, Exceptions2Package::constructorErrorLeavesObjectInInvalidStateQuery()) and
not isNoExceptTrue(c) and
// Constructor must exit with an exception
c = throwingExpr.getConstructor() and
throwingExpr.hasExceptionFlowReflexive(exceptionSource, functionNode, _) and
throwingExpr.hasExceptionFlowReflexive(exceptionSource, throwingExprFlowNode, _) and
exists(ExceptionFlowNode mid |
edges*(exceptionSource, mid) and
newExpr.getASuccessor+() = mid.asThrowingExpr() and
edges*(mid, functionNode) and
edges*(mid, throwingExprFlowNode) and
not exists(ExceptionFlowNode prior | edges(prior, mid) |
prior.asCatchBlock().getEnclosingFunction() = c
)
Expand All @@ -123,7 +130,16 @@ where
DataFlow::localFlow(DataFlow::exprNode(newExpr), DataFlow::exprNode(deletedExpr)) and
newExpr.getASuccessor+() = deletedExpr and
deletedExpr.getASuccessor+() = throwingExpr
)
select c, exceptionSource, functionNode, "Constructor throws $@ and allocates memory at $@",
) and
// In CodeQL CLI 2.12.7 there is a bug which causes an infinite loop during results interpretation
// when a result includes more than maxPaths paths and also includes a path with no edges i.e.
// where the source and sink node are the same.
// To avoid this edge case, if we report a path where the source and sink are the same (i.e the
// throwingExpr directly throws an exception), we adjust the sink node to report the constructor,
// which creates a one step path from the throwingExprFlowNode to the constructor node.
if throwingExprFlowNode = exceptionSource
then reportingNode.asFunction() = c and edges(throwingExprFlowNode, reportingNode)
else reportingNode = throwingExprFlowNode
select c, exceptionSource, reportingNode, "Constructor throws $@ and allocates memory at $@",
throwingExpr, throwingExpr.(ThrowingExpr).getAnExceptionType().getExceptionName(), newExpr,
"alloc"
4 changes: 3 additions & 1 deletion cpp/autosar/test/codeql-pack.lock.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
lockVersion: 1.0.0
dependencies:
codeql/cpp-all:
version: 0.4.6
version: 0.6.1
codeql/ssa:
version: 0.0.14
codeql/tutorial:
version: 0.0.7
compiled: false
Loading
Loading