Skip to content

Commit 038cca8

Browse files
erik-krogherik-krogh
committed
Merge branch 'main' into ts4
2 parents eb84f97 + afe234d commit 038cca8

File tree

168 files changed

+5381
-1288
lines changed

Some content is hidden

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

168 files changed

+5381
-1288
lines changed

‎change-notes/1.26/analysis-javascript.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
| **Query** | **Expected impact** | **Change** |
2828
|--------------------------------|------------------------------|---------------------------------------------------------------------------|
2929
| Incomplete URL substring sanitization (`js/incomplete-url-substring-sanitization`) | More results | This query now recognizes additional URLs when the substring check is an inclusion check. |
30+
| Ambiguous HTML id attribute (`js/duplicate-html-id`) | Results no longer shown | Precision tag reduced to "low". The query is no longer run by default. |
3031

3132

3233
## Changes to libraries

‎change-notes/1.26/analysis-python.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Improvements to Python analysis
2+
3+
The following changes in version 1.26 affect Python analysis in all applications.
4+
5+
## General improvements
6+
7+
8+
## New queries
9+
10+
| **Query** | **Tags** | **Purpose** |
11+
|-----------------------------|-----------|--------------------------------------------------------------------|
12+
13+
14+
## Changes to existing queries
15+
16+
| **Query** | **Expected impact** | **Change** |
17+
|----------------------------|------------------------|------------------------------------------------------------------|
18+
19+
20+
## Changes to libraries
21+
22+
* Added taint tracking support for string formatting through f-strings.

‎config/identical-files.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,10 @@
325325
"csharp/ql/src/experimental/ir/implementation/raw/gvn/internal/ValueNumberingImports.qll",
326326
"csharp/ql/src/experimental/ir/implementation/unaliased_ssa/gvn/internal/ValueNumberingImports.qll"
327327
],
328+
"Inline Test Expectations": [
329+
"cpp/ql/test/TestUtilities/InlineExpectationsTest.qll",
330+
"python/ql/test/TestUtilities/InlineExpectationsTest.qll"
331+
],
328332
"XML": [
329333
"cpp/ql/src/semmle/code/cpp/XML.qll",
330334
"csharp/ql/src/semmle/code/csharp/XML.qll",

‎cpp/ql/src/experimental/Likely Bugs/RedundantNullCheckParam.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
void test(char *arg1, int *arg2) {
2+
if (arg1[0] == 'A') {
3+
if (arg2 != NULL) { //maybe redundant
4+
*arg2 = 42;
5+
}
6+
}
7+
if (arg1[1] == 'B')
8+
{
9+
*arg2 = 54; //dereferenced without checking first
10+
}
11+
}

‎cpp/ql/src/experimental/Likely Bugs/RedundantNullCheckParam.qhelp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>This rule finds comparisons of a function parameter to null that occur when in another path the parameter is dereferenced without a guard check. It's
8+
likely either the check is not required and can be removed, or it should be added before the dereference
9+
so that a null pointer dereference does not occur.</p>
10+
</overview>
11+
12+
<recommendation>
13+
<p>A check should be added to before the dereference, in a way that prevents a null pointer value from
14+
being dereferenced. If it's clear that the pointer cannot be null, consider removing the check instead.</p>
15+
</recommendation>
16+
17+
<example>
18+
<sample src="RedundantNullCheckParam.cpp" />
19+
</example>
20+
21+
<references>
22+
<li>
23+
<a href="https://www.owasp.org/index.php/Null_Dereference">
24+
Null Dereference
25+
</a>
26+
</li>
27+
</references>
28+
29+
</qhelp>

‎cpp/ql/src/experimental/Likely Bugs/RedundantNullCheckParam.ql

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/**
2+
* @name Redundant null check or missing null check of parameter
3+
* @description Checking a parameter for nullness in one path,
4+
* and not in another is likely to be a sign that either
5+
* the check can be removed, or added in the other case.
6+
* @kind problem
7+
* @id cpp/redundant-null-check-param
8+
* @problem.severity recommendation
9+
* @tags reliability
10+
* security
11+
* external/cwe/cwe-476
12+
*/
13+
14+
import cpp
15+
16+
predicate blockDominates(Block check, Block access) {
17+
check.getLocation().getStartLine() <= access.getLocation().getStartLine() and
18+
check.getLocation().getEndLine() >= access.getLocation().getEndLine()
19+
}
20+
21+
predicate isCheckedInstruction(VariableAccess unchecked, VariableAccess checked) {
22+
checked = any(VariableAccess va | va.getTarget() = unchecked.getTarget()) and
23+
//Simple test if the first access in this code path is dereferenced
24+
not dereferenced(checked) and
25+
blockDominates(checked.getEnclosingBlock(), unchecked.getEnclosingBlock())
26+
}
27+
28+
predicate candidateResultUnchecked(VariableAccess unchecked) {
29+
not isCheckedInstruction(unchecked, _)
30+
}
31+
32+
predicate candidateResultChecked(VariableAccess check, EqualityOperation eqop) {
33+
//not dereferenced to check against pointer, not its pointed value
34+
not dereferenced(check) and
35+
//assert macros are not taken into account
36+
not check.isInMacroExpansion() and
37+
// is part of a comparison against some constant NULL
38+
eqop.getAnOperand() = check and
39+
eqop.getAnOperand() instanceof NullValue
40+
}
41+
42+
from VariableAccess unchecked, VariableAccess check, EqualityOperation eqop, Parameter param
43+
where
44+
// a dereference
45+
dereferenced(unchecked) and
46+
// for a function parameter
47+
unchecked.getTarget() = param and
48+
// this function parameter is not overwritten
49+
count(param.getAnAssignment()) = 0 and
50+
check.getTarget() = param and
51+
// which is once checked
52+
candidateResultChecked(check, eqop) and
53+
// and which has not been checked before in this code path
54+
candidateResultUnchecked(unchecked)
55+
select check, "This null check is redundant or there is a missing null check before $@ ", unchecked,
56+
"where dereferencing happens"

‎cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,17 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
484484
// Expr -> Expr
485485
exprToExprStep_nocfg(nodeFrom.asExpr(), nodeTo.asExpr())
486486
or
487+
// Assignment -> LValue post-update node
488+
//
489+
// This is used for assignments whose left-hand side is not a variable
490+
// assignment or a storeStep but is still modeled by other means. It could be
491+
// a call to `operator*` or `operator[]` where taint should flow to the
492+
// post-update node of the qualifier.
493+
exists(AssignExpr assign |
494+
nodeFrom.asExpr() = assign and
495+
nodeTo.(PostUpdateNode).getPreUpdateNode().asExpr() = assign.getLValue()
496+
)
497+
or
487498
// Node -> FlowVar -> VariableAccess
488499
exists(FlowVar var |
489500
(

‎cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,19 @@ predicate localAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeT
8282
exprToDefinitionByReferenceStep(nodeFrom.asExpr(), nodeTo.asDefiningArgument())
8383
or
8484
exprToPartialDefinitionStep(nodeFrom.asExpr(), nodeTo.asPartialDefinition())
85+
or
86+
// Reverse taint: taint that flows from the post-update node of a reference
87+
// returned by a function call, back into the qualifier of that function.
88+
// This allows taint to flow 'in' through references returned by a modeled
89+
// function such as `operator[]`.
90+
exists(TaintFunction f, Call call, FunctionInput inModel, FunctionOutput outModel |
91+
call.getTarget() = f and
92+
inModel.isReturnValueDeref() and
93+
outModel.isQualifierObject() and
94+
f.hasTaintFlow(inModel, outModel) and
95+
nodeFrom.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = call and
96+
nodeTo.asDefiningArgument() = call.getQualifier()
97+
)
8598
}
8699

87100
/**

‎cpp/ql/src/semmle/code/cpp/models/implementations/StdContainer.qll

Lines changed: 70 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,7 @@ import semmle.code.cpp.models.interfaces.Taint
1414
*/
1515
class StdSequenceContainerConstructor extends Constructor, TaintFunction {
1616
StdSequenceContainerConstructor() {
17-
this.getDeclaringType().hasQualifiedName("std", "vector") or
18-
this.getDeclaringType().hasQualifiedName("std", "deque") or
19-
this.getDeclaringType().hasQualifiedName("std", "list") or
20-
this.getDeclaringType().hasQualifiedName("std", "forward_list")
17+
this.getDeclaringType().hasQualifiedName("std", ["vector", "deque", "list", "forward_list"])
2118
}
2219

2320
/**
@@ -26,7 +23,7 @@ class StdSequenceContainerConstructor extends Constructor, TaintFunction {
2623
*/
2724
int getAValueTypeParameterIndex() {
2825
getParameter(result).getUnspecifiedType().(ReferenceType).getBaseType() =
29-
getDeclaringType().getTemplateArgument(0) // i.e. the `T` of this `std::vector<T>`
26+
getDeclaringType().getTemplateArgument(0).(Type).getUnspecifiedType() // i.e. the `T` of this `std::vector<T>`
3027
}
3128

3229
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -36,16 +33,32 @@ class StdSequenceContainerConstructor extends Constructor, TaintFunction {
3633
}
3734
}
3835

36+
/**
37+
* The standard container function `data`.
38+
*/
39+
class StdSequenceContainerData extends TaintFunction {
40+
StdSequenceContainerData() { this.hasQualifiedName("std", ["array", "vector"], "data") }
41+
42+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
43+
// flow from container itself (qualifier) to return value
44+
input.isQualifierObject() and
45+
output.isReturnValueDeref()
46+
or
47+
// reverse flow from returned reference to the qualifier (for writes to
48+
// `data`)
49+
input.isReturnValueDeref() and
50+
output.isQualifierObject()
51+
}
52+
}
53+
3954
/**
4055
* The standard container functions `push_back` and `push_front`.
4156
*/
4257
class StdSequenceContainerPush extends TaintFunction {
4358
StdSequenceContainerPush() {
4459
this.hasQualifiedName("std", "vector", "push_back") or
45-
this.hasQualifiedName("std", "deque", "push_back") or
46-
this.hasQualifiedName("std", "deque", "push_front") or
47-
this.hasQualifiedName("std", "list", "push_back") or
48-
this.hasQualifiedName("std", "list", "push_front") or
60+
this.hasQualifiedName("std", "deque", ["push_back", "push_front"]) or
61+
this.hasQualifiedName("std", "list", ["push_back", "push_front"]) or
4962
this.hasQualifiedName("std", "forward_list", "push_front")
5063
}
5164

@@ -61,14 +74,10 @@ class StdSequenceContainerPush extends TaintFunction {
6174
*/
6275
class StdSequenceContainerFrontBack extends TaintFunction {
6376
StdSequenceContainerFrontBack() {
64-
this.hasQualifiedName("std", "array", "front") or
65-
this.hasQualifiedName("std", "array", "back") or
66-
this.hasQualifiedName("std", "vector", "front") or
67-
this.hasQualifiedName("std", "vector", "back") or
68-
this.hasQualifiedName("std", "deque", "front") or
69-
this.hasQualifiedName("std", "deque", "back") or
70-
this.hasQualifiedName("std", "list", "front") or
71-
this.hasQualifiedName("std", "list", "back") or
77+
this.hasQualifiedName("std", "array", ["front", "back"]) or
78+
this.hasQualifiedName("std", "vector", ["front", "back"]) or
79+
this.hasQualifiedName("std", "deque", ["front", "back"]) or
80+
this.hasQualifiedName("std", "list", ["front", "back"]) or
7281
this.hasQualifiedName("std", "forward_list", "front")
7382
}
7483

@@ -79,16 +88,36 @@ class StdSequenceContainerFrontBack extends TaintFunction {
7988
}
8089
}
8190

91+
/**
92+
* The standard container function `assign`.
93+
*/
94+
class StdSequenceContainerAssign extends TaintFunction {
95+
StdSequenceContainerAssign() {
96+
this.hasQualifiedName("std", ["vector", "deque", "list", "forward_list"], "assign")
97+
}
98+
99+
/**
100+
* Gets the index of a parameter to this function that is a reference to the
101+
* value type of the container.
102+
*/
103+
int getAValueTypeParameterIndex() {
104+
getParameter(result).getUnspecifiedType().(ReferenceType).getBaseType() =
105+
getDeclaringType().getTemplateArgument(0).(Type).getUnspecifiedType() // i.e. the `T` of this `std::vector<T>`
106+
}
107+
108+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
109+
// flow from parameter to string itself (qualifier) and return value
110+
input.isParameterDeref(getAValueTypeParameterIndex()) and
111+
output.isQualifierObject()
112+
}
113+
}
114+
82115
/**
83116
* The standard container `swap` functions.
84117
*/
85118
class StdSequenceContainerSwap extends TaintFunction {
86119
StdSequenceContainerSwap() {
87-
this.hasQualifiedName("std", "array", "swap") or
88-
this.hasQualifiedName("std", "vector", "swap") or
89-
this.hasQualifiedName("std", "deque", "swap") or
90-
this.hasQualifiedName("std", "list", "swap") or
91-
this.hasQualifiedName("std", "forward_list", "swap")
120+
this.hasQualifiedName("std", ["array", "vector", "deque", "list", "forward_list"], "swap")
92121
}
93122

94123
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -100,3 +129,22 @@ class StdSequenceContainerSwap extends TaintFunction {
100129
output.isQualifierObject()
101130
}
102131
}
132+
133+
/**
134+
* The standard container functions `at` and `operator[]`.
135+
*/
136+
class StdSequenceContainerAt extends TaintFunction {
137+
StdSequenceContainerAt() {
138+
this.hasQualifiedName("std", ["vector", "array", "deque"], ["at", "operator[]"])
139+
}
140+
141+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
142+
// flow from qualifier to referenced return value
143+
input.isQualifierObject() and
144+
output.isReturnValueDeref()
145+
or
146+
// reverse flow from returned reference to the qualifier
147+
input.isReturnValueDeref() and
148+
output.isQualifierObject()
149+
}
150+
}

0 commit comments

Comments
 (0)