Skip to content

Commit e040619

Browse files
author
Robert Marsh
committed
Merge branch 'master' into getPhiOperandDefinition-perf-2
2 parents c942da5 + 4efc418 commit e040619

File tree

259 files changed

+27880
-10868
lines changed

Some content is hidden

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

259 files changed

+27880
-10868
lines changed

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,5 @@
1515

1616
# It's useful (though not required) to be able to unpack codeql in the ql checkout itself
1717
/codeql/
18+
.vscode/settings.json
19+
csharp/extractor/Semmle.Extraction.CSharp.Driver/Properties/launchSettings.json

CONTRIBUTING.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ Follow the steps below to help other users understand what your query does, and
1515

1616
2. **Format your code correctly**
1717

18-
All of Semmle's standard queries and libraries are uniformly formatted for clarity and consistency, so we strongly recommend that all contributions follow the same formatting guidelines. If you use QL for Eclipse, you can auto-format your query in the [QL editor](https://help.semmle.com/ql-for-eclipse/Content/WebHelp/ql-editor.html). For more information, see the [CodeQL style guide](https://github.com/Semmle/ql/blob/master/docs/ql-style-guide.md).
18+
All of Semmle's standard queries and libraries are uniformly formatted for clarity and consistency, so we strongly recommend that all contributions follow the same formatting guidelines. If you use CodeQL for VS Code, you can autoformat your query in the [Editor](https://help.semmle.com/codeql/codeql-for-vscode/reference/editor.html#autoformatting). For more information, see the [CodeQL style guide](https://github.com/Semmle/ql/blob/master/docs/ql-style-guide.md).
1919

2020
3. **Make sure your query has the correct metadata**
2121

@@ -26,7 +26,7 @@ Follow the steps below to help other users understand what your query does, and
2626

2727
4. **Make sure the `select` statement is compatible with the query type**
2828

29-
The `select` statement of your query must be compatible with the query type (determined by the `@kind` metadata property) for alert or path results to be displayed correctly in LGTM and QL for Eclipse.
29+
The `select` statement of your query must be compatible with the query type (determined by the `@kind` metadata property) for alert or path results to be displayed correctly in LGTM and CodeQL for VS Code.
3030
For more information on `select` statement format, see [Introduction to query files](https://help.semmle.com/QL/learn-ql/writing-queries/introduction-to-queries.html#select-clause) on help.semmle.com.
3131

3232
5. **Save your query in a `.ql` file in the correct language directory in this repository**

change-notes/1.23/analysis-java.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ The following changes in version 1.23 affect Java analysis in all applications.
77
| **Query** | **Tags** | **Purpose** |
88
|-----------------------------|-----------|--------------------------------------------------------------------|
99
| Continue statement that does not continue (`java/continue-in-false-loop`) | correctness | Finds `continue` statements in `do { ... } while (false)` loops. Results are shown on LGTM by default. |
10+
| Disabled Netty HTTP header validation (`java/netty-http-response-splitting`) | security, external/cwe/cwe-113 | Finds response-splitting vulnerabilities due to Netty HTTP header validation being disabled. Results are shown on LGTM by default. |
1011

1112
## Changes to existing queries
1213

change-notes/1.24/analysis-cpp.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@ The following changes in version 1.24 affect C/C++ analysis in all applications.
1717
| No space for zero terminator (`cpp/no-space-for-terminator`) | More true positive results | This query now identifies a wider variety of buffer allocations using the `semmle.code.cpp.models.interfaces.Allocation` library. |
1818
| Memory is never freed (`cpp/memory-never-freed`) | More true positive results | This query now identifies a wider variety of buffer allocations using the `semmle.code.cpp.models.interfaces.Allocation` library. |
1919
| Memory may not be freed (`cpp/memory-may-not-be-freed`) | More true positive results | This query now identifies a wider variety of buffer allocations using the `semmle.code.cpp.models.interfaces.Allocation` library. |
20+
| Missing return statement (`cpp/missing-return`) | Fewer false positive results | Functions containing `asm` statements are no longer highlighted by this query. |
2021
| Hard-coded Japanese era start date (`cpp/japanese-era/exact-era-date`) | | This query is no longer run on LGTM. |
2122
| No space for zero terminator (`cpp/no-space-for-terminator`) | Fewer false positive results | This query has been modified to be more conservative when identifying which pointers point to null-terminated strings. This approach produces fewer, more accurate results. |
23+
| Overloaded assignment does not return 'this' (`cpp/assignment-does-not-return-this`) | Fewer false positive results | This query no longer reports incorrect results in template classes. |
2224
| Unsafe array for days of the year (`cpp/leap-year/unsafe-array-for-days-of-the-year`) | | This query is no longer run on LGTM. |
2325

2426
## Changes to libraries

change-notes/1.24/analysis-csharp.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,20 @@ The following changes in version 1.24 affect C# analysis in all applications.
1313

1414
| **Query** | **Expected impact** | **Change** |
1515
|------------------------------|------------------------|-----------------------------------|
16+
| Useless assignment to local variable (`cs/useless-assignment-to-local`) | Fewer false positive results | Results have been removed when the variable is named `_` in a `foreach` statement. |
1617

1718
## Removal of old queries
1819

1920
## Changes to code extraction
2021

22+
* Tuple expressions, for example `(int,bool)` in `default((int,bool))` are now extracted correctly.
23+
* Expression nullability flow state is extracted.
24+
2125
## Changes to libraries
2226

2327
* The taint tracking library now tracks flow through (implicit or explicit) conversion operator calls.
2428
* [Code contracts](https://docs.microsoft.com/en-us/dotnet/framework/debug-trace-profile/code-contracts) are now recognized, and are treated like any other assertion methods.
29+
* Expression nullability flow state is given by the predicates `Expr.hasNotNullFlowState()` and `Expr.hasMaybeNullFlowState()`.
2530

2631
## Changes to autobuilder
2732

change-notes/1.24/analysis-java.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,23 @@
22

33
The following changes in version 1.24 affect Java analysis in all applications.
44

5+
## General improvements
6+
7+
* Alert suppression can now be done with single-line block comments (`/* ... */`) as well as line comments (`// ...`).
8+
59
## New queries
610

711
| **Query** | **Tags** | **Purpose** |
812
|-----------------------------|-----------|--------------------------------------------------------------------|
13+
| Failure to use HTTPS or SFTP URL in Maven artifact upload/download (`java/maven/non-https-url`) | security, external/cwe/cwe-300, external/cwe/cwe-319, external/cwe/cwe-494, external/cwe/cwe-829 | Finds use of insecure protocols during Maven dependency resolution. Results are shown on LGTM by default. |
914

1015
## Changes to existing queries
1116

1217
| **Query** | **Expected impact** | **Change** |
1318
|------------------------------|------------------------|-----------------------------------|
19+
| Dereferenced variable may be null (`java/dereferenced-value-may-be-null`) | Fewer false positives | Final fields with a non-null initializer are no longer reported. |
1420
| Expression always evaluates to the same value (`java/evaluation-to-constant`) | Fewer false positives | Expressions of the form `0 * x` are usually intended and no longer reported. |
21+
| Useless null check (`java/useless-null-check`) | More true positives | Useless checks on final fields with a non-null initializer are now reported. |
1522

1623
## Changes to libraries
1724

change-notes/1.24/analysis-javascript.md

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,37 @@
22

33
## General improvements
44

5+
* Alert suppression can now be done with single-line block comments (`/* ... */`) as well as line comments (`// ...`).
6+
7+
* Imports with the `.js` extension can now be resolved to a TypeScript file,
8+
when the import refers to a file generated by TypeScript.
9+
10+
- The analysis of sanitizer guards has improved, leading to fewer false-positive results from the security queries.
11+
512
* Support for the following frameworks and libraries has been improved:
613
- [react](https://www.npmjs.com/package/react)
714
- [typeahead.js](https://www.npmjs.com/package/typeahead.js)
815
- [Handlebars](https://www.npmjs.com/package/handlebars)
916

10-
- Imports with the `.js` extension can now be resolved to a TypeScript file,
11-
when the import refers to a file generated by TypeScript.
12-
1317
## New queries
1418

1519
| **Query** | **Tags** | **Purpose** |
1620
|---------------------------------------------------------------------------------|-------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
1721
| Cross-site scripting through exception (`js/xss-through-exception`) | security, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights potential XSS vulnerabilities where an exception is written to the DOM. Results are not shown on LGTM by default. |
22+
| Regular expression always matches (`js/regex/always-matches`) | correctness, regular-expressions | Highlights regular expression checks that trivially succeed by matching an empty substring. Results are shown on LGTM by default. |
23+
| Missing await (`js/missing-await`) | correctness | Highlights expressions that operate directly on a promise object in a nonsensical way, instead of awaiting its result. Results are shown on LGTM by default. |
24+
| Prototype pollution in utility function (`js/prototype-pollution-utility`) | security, external/cwe/cwe-400, external/cwe/cwe-471 | Highlights recursive copying operations that are susceptible to prototype pollution. Results are shown on LGTM by default. |
1825

1926
## Changes to existing queries
2027

2128
| **Query** | **Expected impact** | **Change** |
2229
|--------------------------------|------------------------------|---------------------------------------------------------------------------|
2330
| Clear-text logging of sensitive information (`js/clear-text-logging`) | More results | More results involving `process.env` and indirect calls to logging methods are recognized. |
31+
| Duplicate parameter names (`js/duplicate-parameter-name`) | Fewer results | This query now recognizes additional parameters that reasonably can have duplicated names. |
2432
| Incomplete string escaping or encoding (`js/incomplete-sanitization`) | Fewer false positive results | This query now recognizes additional cases where a single replacement is likely to be intentional. |
2533
| Unbound event handler receiver (`js/unbound-event-handler-receiver`) | Fewer false positive results | This query now recognizes additional ways event handler receivers can be bound. |
34+
| Expression has no effect (`js/useless-expression`) | Fewer false positive results | The query now recognizes block-level flow type annotations and ignores the first statement of a try block. |
35+
| Use of call stack introspection in strict mode (`js/strict-mode-call-stack-introspection`) | Fewer false positive results | The query no longer flags expression statements. |
2636

2737
## Changes to libraries
2838

cpp/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
import cpp
2020
import semmle.code.cpp.controlflow.SSA
21+
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
22+
import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
2123

2224
/**
2325
* Holds if `e` is either:
@@ -64,6 +66,110 @@ int getEffectiveMulOperands(MulExpr me) {
6466
)
6567
}
6668

69+
/**
70+
* As SimpleRangeAnalysis does not support reasoning about multiplication
71+
* we create a tiny abstract interpreter for handling multiplication, which
72+
* we invoke only after weeding out of all of trivial cases that we do
73+
* not care about. By default, the maximum and minimum values are computed
74+
* using SimpleRangeAnalysis.
75+
*/
76+
class AnalyzableExpr extends Expr {
77+
float maxValue() { result = upperBound(this.getFullyConverted()) }
78+
79+
float minValue() { result = lowerBound(this.getFullyConverted()) }
80+
}
81+
82+
class ParenAnalyzableExpr extends AnalyzableExpr, ParenthesisExpr {
83+
override float maxValue() { result = this.getExpr().(AnalyzableExpr).maxValue() }
84+
85+
override float minValue() { result = this.getExpr().(AnalyzableExpr).minValue() }
86+
}
87+
88+
class MulAnalyzableExpr extends AnalyzableExpr, MulExpr {
89+
override float maxValue() {
90+
exists(float x1, float y1, float x2, float y2 |
91+
x1 = this.getLeftOperand().getFullyConverted().(AnalyzableExpr).minValue() and
92+
x2 = this.getLeftOperand().getFullyConverted().(AnalyzableExpr).maxValue() and
93+
y1 = this.getRightOperand().getFullyConverted().(AnalyzableExpr).minValue() and
94+
y2 = this.getRightOperand().getFullyConverted().(AnalyzableExpr).maxValue() and
95+
result = (x1 * y1).maximum(x1 * y2).maximum(x2 * y1).maximum(x2 * y2)
96+
)
97+
}
98+
99+
override float minValue() {
100+
exists(float x1, float x2, float y1, float y2 |
101+
x1 = this.getLeftOperand().getFullyConverted().(AnalyzableExpr).minValue() and
102+
x2 = this.getLeftOperand().getFullyConverted().(AnalyzableExpr).maxValue() and
103+
y1 = this.getRightOperand().getFullyConverted().(AnalyzableExpr).minValue() and
104+
y2 = this.getRightOperand().getFullyConverted().(AnalyzableExpr).maxValue() and
105+
result = (x1 * y1).minimum(x1 * y2).minimum(x2 * y1).minimum(x2 * y2)
106+
)
107+
}
108+
}
109+
110+
class AddAnalyzableExpr extends AnalyzableExpr, AddExpr {
111+
override float maxValue() {
112+
result = this.getLeftOperand().getFullyConverted().(AnalyzableExpr).maxValue() +
113+
this.getRightOperand().getFullyConverted().(AnalyzableExpr).maxValue()
114+
}
115+
116+
override float minValue() {
117+
result = this.getLeftOperand().getFullyConverted().(AnalyzableExpr).minValue() +
118+
this.getRightOperand().getFullyConverted().(AnalyzableExpr).minValue()
119+
}
120+
}
121+
122+
class SubAnalyzableExpr extends AnalyzableExpr, SubExpr {
123+
override float maxValue() {
124+
result = this.getLeftOperand().getFullyConverted().(AnalyzableExpr).maxValue() -
125+
this.getRightOperand().getFullyConverted().(AnalyzableExpr).minValue()
126+
}
127+
128+
override float minValue() {
129+
result = this.getLeftOperand().getFullyConverted().(AnalyzableExpr).minValue() -
130+
this.getRightOperand().getFullyConverted().(AnalyzableExpr).maxValue()
131+
}
132+
}
133+
134+
class VarAnalyzableExpr extends AnalyzableExpr, VariableAccess {
135+
VarAnalyzableExpr() { this.getTarget() instanceof StackVariable }
136+
137+
override float maxValue() {
138+
exists(SsaDefinition def, Variable v |
139+
def.getAUse(v) = this and
140+
// if there is a defining expression, use that for
141+
// computing the maximum value. Otherwise, assign the
142+
// variable the largest possible value it can hold
143+
if exists(def.getDefiningValue(v))
144+
then result = def.getDefiningValue(v).(AnalyzableExpr).maxValue()
145+
else result = upperBound(this)
146+
)
147+
}
148+
149+
override float minValue() {
150+
exists(SsaDefinition def, Variable v |
151+
def.getAUse(v) = this and
152+
if exists(def.getDefiningValue(v))
153+
then result = def.getDefiningValue(v).(AnalyzableExpr).minValue()
154+
else result = lowerBound(this)
155+
)
156+
}
157+
}
158+
159+
/**
160+
* Holds if `t` is not an instance of `IntegralType`,
161+
* or if `me` cannot be proven to not overflow
162+
*/
163+
predicate overflows(MulExpr me, Type t) {
164+
t instanceof IntegralType
165+
implies
166+
(
167+
me.(MulAnalyzableExpr).maxValue() > exprMaxVal(me)
168+
or
169+
me.(MulAnalyzableExpr).minValue() < exprMinVal(me)
170+
)
171+
}
172+
67173
from MulExpr me, Type t1, Type t2
68174
where
69175
t1 = me.getType().getUnderlyingType() and
@@ -101,7 +207,11 @@ where
101207
e = other.(BinaryOperation).getAnOperand*()
102208
) and
103209
e.(Literal).getType().getSize() = t2.getSize()
104-
)
210+
) and
211+
// only report if we cannot prove that the result of the
212+
// multiplication will be less (resp. greater) than the
213+
// maximum (resp. minimum) number we can compute.
214+
overflows(me, t1)
105215
select me,
106216
"Multiplication result may overflow '" + me.getType().toString() + "' before it is converted to '"
107217
+ me.getFullyConverted().getType().toString() + "'."

cpp/ql/src/Likely Bugs/InconsistentCheckReturnNull.ql

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,16 @@ predicate assertInvocation(File f, int line) {
2525
)
2626
}
2727

28-
predicate nullCheckAssert(Expr e, Variable v, Declaration qualifier) {
29-
nullCheckInCondition(e, v, qualifier) and
28+
class InterestingExpr extends Expr {
29+
InterestingExpr() { nullCheckInCondition(this, _, _) }
30+
}
31+
32+
predicate nullCheckAssert(InterestingExpr e, Variable v, Declaration qualifier) {
3033
exists(File f, int i |
31-
e.getLocation().getStartLine() = i and e.getFile() = f and assertInvocation(f, i)
34+
e.getLocation().getStartLine() = i and
35+
e.getFile() = f and
36+
assertInvocation(f, i) and
37+
nullCheckInCondition(e, v, qualifier)
3238
)
3339
}
3440

cpp/ql/src/Likely Bugs/Likely Typos/AssignWhereCompareMeant.ql

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,31 @@ abstract class BooleanControllingAssignment extends AssignExpr {
3838
abstract predicate isWhitelisted();
3939
}
4040

41+
/**
42+
* Gets an operand of a logical operation expression (we need the restriction
43+
* to BinaryLogicalOperation expressions to get the correct transitive closure).
44+
*/
45+
Expr getComparisonOperand(BinaryLogicalOperation op) { result = op.getAnOperand() }
46+
4147
class BooleanControllingAssignmentInExpr extends BooleanControllingAssignment {
4248
BooleanControllingAssignmentInExpr() {
4349
this.getParent() instanceof UnaryLogicalOperation or
4450
this.getParent() instanceof BinaryLogicalOperation or
4551
exists(ConditionalExpr c | c.getCondition() = this)
4652
}
4753

48-
override predicate isWhitelisted() { this.getConversion().(ParenthesisExpr).isParenthesised() }
54+
override predicate isWhitelisted() {
55+
this.getConversion().(ParenthesisExpr).isParenthesised()
56+
or
57+
// whitelist this assignment if all comparison operations in the expression that this
58+
// assignment is part of, are not parenthesized. In that case it seems like programmer
59+
// is fine with unparenthesized comparison operands to binary logical operators, and
60+
// the parenthesis around this assignment was used to call it out as an assignment.
61+
this.isParenthesised() and
62+
forex(ComparisonOperation op | op = getComparisonOperand*(this.getParent+()) |
63+
not op.isParenthesised()
64+
)
65+
}
4966
}
5067

5168
class BooleanControllingAssignmentInStmt extends BooleanControllingAssignment {
@@ -65,7 +82,8 @@ class BooleanControllingAssignmentInStmt extends BooleanControllingAssignment {
6582
*/
6683
predicate candidateResult(BooleanControllingAssignment ae) {
6784
ae.getRValue().isConstant() and
68-
not ae.isWhitelisted()
85+
not ae.isWhitelisted() and
86+
not ae.getRValue() instanceof StringLiteral
6987
}
7088

7189
/**
@@ -81,5 +99,6 @@ predicate candidateVariable(Variable v) {
8199
from BooleanControllingAssignment ae, UndefReachability undef
82100
where
83101
candidateResult(ae) and
102+
not ae.isFromUninstantiatedTemplate(_) and
84103
not undef.reaches(_, ae.getLValue().(VariableAccess).getTarget(), ae.getLValue())
85104
select ae, "Use of '=' where '==' may have been intended."

0 commit comments

Comments
 (0)