Skip to content

Commit 3c3cc2e

Browse files
authored
Merge pull request #175 from hvitved/merge-rc
Merge rc/1.18 into master
2 parents b6b3581 + 70e7131 commit 3c3cc2e

File tree

97 files changed

+3624
-3161
lines changed

Some content is hidden

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

97 files changed

+3624
-3161
lines changed

change-notes/1.18/analysis-cpp.md

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,28 @@
11
# Improvements to C/C++ analysis
22

3-
## General improvements
4-
5-
> Changes that affect alerts in many files or from many queries
6-
> For example, changes to file classification
7-
83
## New queries
94

105
| **Query** | **Tags** | **Purpose** |
116
|-----------------------------|-----------|--------------------------------------------------------------------|
12-
| Upcast array used in pointer arithmetic | reliability, correctness, external/cwe/cwe-119 | Finds undefined behavior caused by doing pointer arithmetic on an array of objects that has been cast to an array of a supertype. |
7+
| Upcast array used in pointer arithmetic (`cpp/upcast-array-pointer-arithmetic`) | reliability, correctness, external/cwe/cwe-119 | Finds undefined behavior caused by doing pointer arithmetic on an array of objects that has been cast to an array of a supertype. |
138

149
## Changes to existing queries
1510

1611
| **Query** | **Expected impact** | **Change** |
1712
|----------------------------|------------------------|------------------------------------------------------------------|
18-
| Self comparison | Fewer false positive results | Range checks of the form `x == (T)x` are no longer flagged unless they are guaranteed to have the same result on all platforms. |
19-
| [Nested loops with same variable] | Fewer false positive results | Results where the loop variable is a member of a class or struct now account for the object. |
20-
| [For loop variable changed in body] | Fewer false positive results | Results where the loop variable is a member of a class or struct now account for the object. |
21-
| [Local variable hides global variable] | Fewer false positive results | Results for parameters are now only reported if the name of the global variable is the same as the name of the parameter as used in the function definition (not just a function declaration). |
22-
| [Memory may not be freed] | More correct results | This query now models calls to `realloc` more accurately. |
23-
| Wrong number of arguments to formatting function | Fewer false positive results | Some false positives related to custom printf-like functions have been fixed. |
24-
| Wrong number of arguments to formatting function | Clear separation between results of high and low severity | This query has been split into two queries: a high-severity query named [Too few arguments to formatting function] and a low-severity query named [Too many arguments to formatting function]. |
25-
| [Too few arguments to formatting function] | More correct and fewer false positives results | This query now understands positional format arguments as supported by some libraries. |
26-
| [Too many arguments to formatting function] | More correct and fewer false positives results | This query now understands positional format arguments as supported by some libraries. |
27-
| [Variable used in its own initializer] | Fewer false positive results | Results where a macro is used to indicate deliberate uninitialization are now excluded |
28-
| [Assignment where comparison was intended] | Fewer false positive results | Results are no longer reported if the variable is not yet defined. |
29-
| [Comparison where assignment was intended] | More correct results | "This query now includes results where an overloaded `operator==` is used in the wrong context. |
30-
| [User-controlled data in arithmetic expression] | More correct results | Increment / decrement / addition assignment / subtraction assignment operations are now understood as arithmetic operations in this query. |
31-
| [Uncontrolled data in arithmetic expression] | More correct results | Increment / decrement / addition assignment / subtraction assignment operations are now understood as arithmetic operations in this query. |
32-
| [Use of extreme values in arithmetic expression] | More correct results | Increment / decrement / addition assignment / subtraction assignment operations are now understood as arithmetic operations in this query. |
33-
| [Use of extreme values in arithmetic expression] | Fewer false positives | The query now considers whether a particular expression might cause an overflow of minimum or maximum values only. |
13+
| Assignment where comparison was intended (`cpp/assign-where-compare-meant`) | Fewer false positive results | Results are no longer reported if the variable is not yet defined. |
14+
| Comparison where assignment was intended (`cpp/compare-where-assign-meant`) | More results | This query now includes results where an overloaded `operator==` is used in the wrong context. |
15+
| For loop variable changed in body (`cpp/loop-variable-changed`) | Fewer false positive results | Results where the loop variable is a member of a class or struct now account for the object. |
16+
| Local variable hides global variable (`cpp/local-variable-hides-global-variable`) | Fewer false positive results | Results for parameters are now only reported if the name of the global variable is the same as the name of the parameter as used in the function definition (not just a function declaration). |
17+
| Nested loops with same variable (`cpp/nested-loops-with-same-variable`) | Fewer false positive results | Results where the loop variable is a member of a class or struct now account for the object. |
18+
| Self comparison (`cpp/comparison-of-identical-expressions`) | Fewer false positive results | Range checks of the form `x == (T)x` are no longer flagged unless they are guaranteed to have the same result on all platforms. |
19+
| Too few arguments to formatting function (`cpp/wrong-number-format-arguments`) | More precise results | This was previously known as "Wrong number of arguments to formatting function". It now focuses only on functions calls that are missing arguments, which tend to be more severe. See the next row for the new query that reports lower-severity alerts for calls with too many arguments. In addition, both queries now understand positional format arguments as supported by some libraries, and some false positive results for custom printf-like functions have been fixed.|
20+
| Too many arguments to formatting function (`cpp/too-many-format-arguments`) | More precise results | This new query was created by splitting the old "Wrong number of arguments to formatting function" query (see row above). It reports function calls with too many arguments. |
21+
| User-controlled data in arithmetic expression (`cpp/tainted-arithmetic`) | More results | The query is extended to analyze increment, decrement, addition-assignment, and subtraction-assignment operations. |
22+
| Variable used in its own initializer (`cpp/use-in-own-initializer`) | Fewer false positive results | Results where a macro is used to indicate deliberate uninitialization are now excluded. |
23+
|Uncontrolled data in arithmetic expression (`cpp/uncontrolled-arithmetic`) | More results | The query is extended to analyze increment, decrement, addition-assignment, and subtraction-assignment operations. |
3424

3525
## Changes to QL libraries
3626

37-
* Fixes for aggregate initializers using designators:
38-
* `ClassAggregateLiteral.getFieldExpr()` previously assumed initializer expressions appeared in the same order as the declaration order of the fields, causing it to associate the expressions with the wrong fields when using designated initializers. This has been fixed.
39-
* `ArrayAggregateLiteral.getElementExpr()` previously assumed initializer expressions appeared in the same order as the corresponding array elements, causing it to associate the expressions with the wrong array elements when using designated initializers. This has been fixed.
40-
* `Element.getEnclosingElement()` no longer includes macro accesses in its results. To explore parents and children of macro accesses, use the relevant member predicates on `MacroAccess` or `MacroInvocation`.
27+
* The `ClassAggregateLiteral.getFieldExpr()` and `ArrayAggregateLiteral.getElementExpr()` predicates incorrectly assumed that initializer expressions appeared in the same order as the declaration order of the elements. This resulted in the association of the expressions with the wrong elements when designated initializers were used. This has been fixed.
28+
* Results for the `Element.getEnclosingElement()` predicate no longer included macro accesses. To explore parents and children of macro accesses, use the relevant member predicates on `MacroAccess` or `MacroInvocation`.

change-notes/1.18/analysis-csharp.md

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,31 @@
3636

3737
## Changes to code extraction
3838

39+
* The `into` part of `join` clauses is now extracted.
40+
* The `when` part of constant cases is now extracted.
41+
* Fixed a bug where `while(x is T y) ...` was not extracted correctly.
42+
3943
* *Series of bullet points*
4044

4145
## Changes to QL libraries
4246

43-
* A new non-member predicate `mayBeDisposed()` can be used to determine if a variable is potentially disposed inside a library. It will analyse the CIL code in the library to determine this.
47+
* A new non-member predicate `mayBeDisposed()` can be used to determine if a variable is potentially disposed inside a library. It will analyse the CIL code in the library to determine this.
48+
* Several control flow graph entities have been renamed (the old names still exist for backwards compatibility):
49+
- `ControlFlowNode` has been renamed to `ControlFlow::Node`.
50+
- `CallableEntryNode` has been renamed to `ControlFlow::Nodes::EntryNode`.
51+
- `CallableExitNode` has been renamed to `ControlFlow::Nodes::ExitNode`.
52+
- `ControlFlowEdgeType` has been renamed to `ControlFlow::SuccessorType`.
53+
- `ControlFlowEdgeSuccessor` has been renamed to `ControlFlow::SuccessorTypes::NormalSuccessor`.
54+
- `ControlFlowEdgeConditional` has been renamed to `ControlFlow::SuccessorTypes::ConditionalSuccessor`.
55+
- `ControlFlowEdgeBoolean` has been renamed to `ControlFlow::SuccessorTypes::BooleanSuccessor`.
56+
- `ControlFlowEdgeNullness` has been renamed to `ControlFlow::SuccessorTypes::NullnessSuccessor`.
57+
- `ControlFlowEdgeMatching` has been renamed to `ControlFlow::SuccessorTypes::MatchingSuccessor`.
58+
- `ControlFlowEdgeEmptiness` has been renamed to `ControlFlow::SuccessorTypes::EmptinessSuccessor`.
59+
- `ControlFlowEdgeReturn` has been renamed to `ControlFlow::SuccessorTypes::ReturnSuccessor`.
60+
- `ControlFlowEdgeBreak` has been renamed to `ControlFlow::SuccessorTypes::BreakSuccessor`.
61+
- `ControlFlowEdgeContinue` has been renamed to `ControlFlow::SuccessorTypes::ContinueSuccessor`.
62+
- `ControlFlowEdgeGotoLabel` has been renamed to `ControlFlow::SuccessorTypes::GotoLabelSuccessor`.
63+
- `ControlFlowEdgeGotoCase` has been renamed to `ControlFlow::SuccessorTypes::GotoCaseSuccessor`.
64+
- `ControlFlowEdgeGotoDefault` has been renamed to `ControlFlow::SuccessorTypes::GotoDefaultSuccessor`.
65+
- `ControlFlowEdgeException` has been renamed to `ControlFlow::SuccessorTypes::ExceptionSuccessor`.
66+
* The predicate `getCondition()` has been moved from `TypeCase` to `CaseStmt`. It is now possible to get the condition of a `ConstCase` using its `getCondition()` predicate.

change-notes/1.18/analysis-javascript.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,12 @@
107107
| Hard-coded credentials | More true-positive results | This rule now recognizes secret cryptographic keys. |
108108
| Incomplete string escaping or encoding | Better name, more true-positive results | This rule has been renamed to more clearly reflect its purpose. Also, it now recognizes incomplete URL encoding and decoding. |
109109
| Insecure randomness | More true-positive results | This rule now recognizes secret cryptographic keys. |
110+
| Misleading indentation after control statement | Fewer results | This rule temporarily ignores TypeScript files. |
110111
| Missing rate limiting | More true-positive results, fewer false-positive results | This rule now recognizes additional rate limiters and expensive route handlers. |
111112
| Missing X-Frame-Options HTTP header | Fewer false-positive results | This rule now treats header names case-insensitively. |
113+
| Omitted array element | Fewer results | This rule temporarily ignores TypeScript files. |
112114
| Reflected cross-site scripting | Fewer false-positive results | This rule now treats header names case-insensitively. |
115+
| Semicolon insertion | Fewer results | This rule temporarily ignores TypeScript files. |
113116
| Server-side URL redirect | More true-positive results | This rule now treats header names case-insensitively. |
114117
| Superfluous trailing arguments | Fewer false-positive results | This rule now ignores calls to some empty functions. |
115118
| Type confusion through parameter tampering | Fewer false-positive results | This rule no longer flags emptiness checks. |

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ class ReturnValueInstruction extends ReturnInstruction {
602602
}
603603

604604
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
605-
tag instanceof ReturnValueOperand and
605+
exists(this.getOperand(tag.(ReturnValueOperand))) and
606606
result instanceof IndirectMemoryAccess
607607
}
608608
}
@@ -629,7 +629,7 @@ class LoadInstruction extends CopyInstruction {
629629
}
630630

631631
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
632-
tag instanceof CopySourceOperand and
632+
exists(this.getOperand(tag.(CopySourceOperand))) and
633633
result instanceof IndirectMemoryAccess
634634
}
635635

@@ -1015,7 +1015,7 @@ class ThrowValueInstruction extends ThrowInstruction {
10151015
}
10161016

10171017
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
1018-
tag instanceof ExceptionOperand and
1018+
exists(this.getOperand(tag.(ExceptionOperand))) and
10191019
result instanceof IndirectMemoryAccess
10201020
}
10211021

@@ -1114,7 +1114,7 @@ class UnmodeledUseInstruction extends Instruction {
11141114
}
11151115

11161116
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
1117-
tag instanceof UnmodeledUseOperand and
1117+
exists(this.getOperand(tag.(UnmodeledUseOperand))) and
11181118
result instanceof UnmodeledMemoryAccess
11191119
}
11201120
}
@@ -1125,7 +1125,7 @@ class PhiInstruction extends Instruction {
11251125
}
11261126

11271127
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
1128-
tag instanceof PhiOperand and
1128+
exists(this.getOperand(tag.(PhiOperand))) and
11291129
result instanceof PhiMemoryAccess
11301130
}
11311131

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ class ReturnValueInstruction extends ReturnInstruction {
602602
}
603603

604604
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
605-
tag instanceof ReturnValueOperand and
605+
exists(this.getOperand(tag.(ReturnValueOperand))) and
606606
result instanceof IndirectMemoryAccess
607607
}
608608
}
@@ -629,7 +629,7 @@ class LoadInstruction extends CopyInstruction {
629629
}
630630

631631
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
632-
tag instanceof CopySourceOperand and
632+
exists(this.getOperand(tag.(CopySourceOperand))) and
633633
result instanceof IndirectMemoryAccess
634634
}
635635

@@ -1015,7 +1015,7 @@ class ThrowValueInstruction extends ThrowInstruction {
10151015
}
10161016

10171017
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
1018-
tag instanceof ExceptionOperand and
1018+
exists(this.getOperand(tag.(ExceptionOperand))) and
10191019
result instanceof IndirectMemoryAccess
10201020
}
10211021

@@ -1114,7 +1114,7 @@ class UnmodeledUseInstruction extends Instruction {
11141114
}
11151115

11161116
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
1117-
tag instanceof UnmodeledUseOperand and
1117+
exists(this.getOperand(tag.(UnmodeledUseOperand))) and
11181118
result instanceof UnmodeledMemoryAccess
11191119
}
11201120
}
@@ -1125,7 +1125,7 @@ class PhiInstruction extends Instruction {
11251125
}
11261126

11271127
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
1128-
tag instanceof PhiOperand and
1128+
exists(this.getOperand(tag.(PhiOperand))) and
11291129
result instanceof PhiMemoryAccess
11301130
}
11311131

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ class ReturnValueInstruction extends ReturnInstruction {
602602
}
603603

604604
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
605-
tag instanceof ReturnValueOperand and
605+
exists(this.getOperand(tag.(ReturnValueOperand))) and
606606
result instanceof IndirectMemoryAccess
607607
}
608608
}
@@ -629,7 +629,7 @@ class LoadInstruction extends CopyInstruction {
629629
}
630630

631631
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
632-
tag instanceof CopySourceOperand and
632+
exists(this.getOperand(tag.(CopySourceOperand))) and
633633
result instanceof IndirectMemoryAccess
634634
}
635635

@@ -1015,7 +1015,7 @@ class ThrowValueInstruction extends ThrowInstruction {
10151015
}
10161016

10171017
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
1018-
tag instanceof ExceptionOperand and
1018+
exists(this.getOperand(tag.(ExceptionOperand))) and
10191019
result instanceof IndirectMemoryAccess
10201020
}
10211021

@@ -1114,7 +1114,7 @@ class UnmodeledUseInstruction extends Instruction {
11141114
}
11151115

11161116
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
1117-
tag instanceof UnmodeledUseOperand and
1117+
exists(this.getOperand(tag.(UnmodeledUseOperand))) and
11181118
result instanceof UnmodeledMemoryAccess
11191119
}
11201120
}
@@ -1125,7 +1125,7 @@ class PhiInstruction extends Instruction {
11251125
}
11261126

11271127
override final MemoryAccessKind getOperandMemoryAccess(OperandTag tag) {
1128-
tag instanceof PhiOperand and
1128+
exists(this.getOperand(tag.(PhiOperand))) and
11291129
result instanceof PhiMemoryAccess
11301130
}
11311131

csharp/ql/src/API Abuse/Dispose.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ private import semmle.code.csharp.frameworks.system.web.UI
55

66
class DisposableType extends RefType {
77
DisposableType() {
8-
this.getABaseType+() = getSystemIDisposableInterface()
8+
this.getABaseType+() instanceof SystemIDisposableInterface
99
}
1010
}
1111

@@ -17,13 +17,13 @@ class DisposableField extends Field {
1717

1818
class WebControl extends RefType {
1919
WebControl() {
20-
this.getBaseClass*() = getSystemWebUIControlClass()
20+
this.getBaseClass*() instanceof SystemWebUIControlClass
2121
}
2222
}
2323

2424
class WebPage extends RefType {
2525
WebPage() {
26-
this.getBaseClass*() = getSystemWebUIPageClass()
26+
this.getBaseClass*() instanceof SystemWebUIPageClass
2727
}
2828
}
2929

csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import csharp
1515
import semmle.code.csharp.commons.Assertions
1616
import semmle.code.csharp.commons.Constants
17-
import ControlFlowGraph
1817

1918
/** A constant condition. */
2019
abstract class ConstantCondition extends Expr {
@@ -76,13 +75,13 @@ class ConstantNullnessCondition extends ConstantCondition {
7675
boolean b;
7776

7877
ConstantNullnessCondition() {
79-
forex(ControlFlowNode cfn |
78+
forex(ControlFlow::Node cfn |
8079
cfn = this.getAControlFlowNode() |
81-
exists(ControlFlowEdgeNullness t |
80+
exists(ControlFlow::SuccessorTypes::NullnessSuccessor t |
8281
exists(cfn.getASuccessorByType(t)) |
8382
if t.isNull() then b = true else b = false
8483
) and
85-
strictcount(ControlFlowEdgeType t | exists(cfn.getASuccessorByType(t))) = 1
84+
strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1
8685
)
8786
}
8887

@@ -99,13 +98,13 @@ class ConstantMatchingCondition extends ConstantCondition {
9998
boolean b;
10099

101100
ConstantMatchingCondition() {
102-
forex(ControlFlowNode cfn |
101+
forex(ControlFlow::Node cfn |
103102
cfn = this.getAControlFlowNode() |
104-
exists(ControlFlowEdgeMatching t |
103+
exists(ControlFlow::SuccessorTypes::MatchingSuccessor t |
105104
exists(cfn.getASuccessorByType(t)) |
106105
if t.isMatch() then b = true else b = false
107106
) and
108-
strictcount(ControlFlowEdgeType t | exists(cfn.getASuccessorByType(t))) = 1
107+
strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1
109108
)
110109
}
111110

csharp/ql/src/Bad Practices/ErroneousClassCompare.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,5 @@ class StringComparison extends Expr {
3535
from StringComparison sc, PropertyAccess pa
3636
where sc.getAnOperand() instanceof StringLiteral
3737
and sc.getAnOperand() = pa
38-
and pa.getTarget() = getSystemTypeClass().getFullNameProperty()
38+
and pa.getTarget() = any(SystemTypeClass c).getFullNameProperty()
3939
select sc, "Erroneous class compare."

0 commit comments

Comments
 (0)