Skip to content

Commit 4f2060f

Browse files
committed
Merge commit '2d618d6b928d8b76ac8033b3b63d9bde71caa325' into yo-h/java16
2 parents cc63563 + 2d618d6 commit 4f2060f

File tree

148 files changed

+3743
-330
lines changed

Some content is hidden

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

148 files changed

+3743
-330
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* The 'Assignment where comparison was intended' (cpp/assign-where-compare-meant) query has been improved to flag fewer benign assignments in conditionals.

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,29 @@ class BooleanControllingAssignmentInExpr extends BooleanControllingAssignment {
5454
override predicate isWhitelisted() {
5555
this.getConversion().(ParenthesisExpr).isParenthesised()
5656
or
57-
// whitelist this assignment if all comparison operations in the expression that this
57+
// Allow this assignment if all comparison operations in the expression that this
5858
// assignment is part of, are not parenthesized. In that case it seems like programmer
5959
// is fine with unparenthesized comparison operands to binary logical operators, and
6060
// the parenthesis around this assignment was used to call it out as an assignment.
6161
this.isParenthesised() and
6262
forex(ComparisonOperation op | op = getComparisonOperand*(this.getParent+()) |
6363
not op.isParenthesised()
6464
)
65+
or
66+
// Match a pattern like:
67+
// ```
68+
// if((a = b) && use_value(a)) { ... }
69+
// ```
70+
// where the assignment is meant to update the value of `a` before it's used in some other boolean
71+
// subexpression that is guarenteed to be evaluate _after_ the assignment.
72+
this.isParenthesised() and
73+
exists(LogicalAndExpr parent, Variable var, VariableAccess access |
74+
var = this.getLValue().(VariableAccess).getTarget() and
75+
access = var.getAnAccess() and
76+
not access.isUsedAsLValue() and
77+
parent.getRightOperand() = access.getParent*() and
78+
parent.getLeftOperand() = this.getParent*()
79+
)
6580
}
6681
}
6782

cpp/ql/src/Security/CWE/CWE-428/UnsafeCreateProcessCall.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ class QuotedCommandInCreateProcessFunctionConfiguration extends DataFlow2::Confi
9393

9494
bindingset[s]
9595
predicate isQuotedOrNoSpaceApplicationNameOnCmd(string s) {
96-
s.regexpMatch("\"([^\"])*\"(\\s|.)*") // The first element (path) is quoted
96+
s.regexpMatch("\"([^\"])*\"[\\s\\S]*") // The first element (path) is quoted
9797
or
9898
s.regexpMatch("[^\\s]+") // There are no spaces in the string
9999
}

cpp/ql/src/semmle/code/cpp/Location.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ class Location extends @location {
7272
}
7373

7474
/** Holds if `this` comes on a line strictly before `l`. */
75+
pragma[inline]
7576
predicate isBefore(Location l) {
7677
this.getFile() = l.getFile() and this.getEndLine() < l.getStartLine()
7778
}

cpp/ql/src/semmle/code/cpp/models/implementations/SmartPointer.qll

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import semmle.code.cpp.models.interfaces.Taint
2+
import semmle.code.cpp.models.interfaces.DataFlow
23
import semmle.code.cpp.models.interfaces.PointerWrapper
34

45
/**
@@ -14,6 +15,23 @@ private class UniqueOrSharedPtr extends Class, PointerWrapper {
1415
}
1516
}
1617

18+
/** Any function that unwraps a pointer wrapper class to reveal the underlying pointer. */
19+
private class PointerWrapperDataFlow extends DataFlowFunction {
20+
PointerWrapperDataFlow() {
21+
this = any(PointerWrapper wrapper).getAnUnwrapperFunction() and
22+
not this.getUnspecifiedType() instanceof ReferenceType
23+
}
24+
25+
override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
26+
input.isQualifierAddress() and output.isReturnValue()
27+
or
28+
input.isQualifierObject() and output.isReturnValueDeref()
29+
or
30+
input.isReturnValueDeref() and
31+
output.isQualifierObject()
32+
}
33+
}
34+
1735
/**
1836
* The `std::make_shared` and `std::make_unique` template functions.
1937
*/

cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3260,15 +3260,59 @@
32603260
| smart_pointer.cpp:47:11:47:11 | p | smart_pointer.cpp:47:10:47:10 | call to operator* | TAINT |
32613261
| smart_pointer.cpp:51:30:51:50 | call to make_shared | smart_pointer.cpp:52:10:52:10 | p | |
32623262
| smart_pointer.cpp:51:52:51:57 | call to source | smart_pointer.cpp:51:30:51:50 | call to make_shared | TAINT |
3263-
| smart_pointer.cpp:52:10:52:10 | p | smart_pointer.cpp:52:12:52:14 | call to get | TAINT |
3263+
| smart_pointer.cpp:52:10:52:10 | p | smart_pointer.cpp:52:12:52:14 | call to get | |
3264+
| smart_pointer.cpp:52:12:52:14 | ref arg call to get | smart_pointer.cpp:52:10:52:10 | ref arg p | |
32643265
| smart_pointer.cpp:56:30:56:50 | call to make_unique | smart_pointer.cpp:57:10:57:10 | p | |
32653266
| smart_pointer.cpp:56:52:56:57 | call to source | smart_pointer.cpp:56:30:56:50 | call to make_unique | TAINT |
3266-
| smart_pointer.cpp:57:10:57:10 | p | smart_pointer.cpp:57:12:57:14 | call to get | TAINT |
3267+
| smart_pointer.cpp:57:10:57:10 | p | smart_pointer.cpp:57:12:57:14 | call to get | |
3268+
| smart_pointer.cpp:57:12:57:14 | ref arg call to get | smart_pointer.cpp:57:10:57:10 | ref arg p | |
32673269
| smart_pointer.cpp:65:28:65:46 | call to make_unique | smart_pointer.cpp:66:10:66:10 | p | |
32683270
| smart_pointer.cpp:65:28:65:46 | call to make_unique | smart_pointer.cpp:67:10:67:10 | p | |
32693271
| smart_pointer.cpp:65:48:65:53 | call to source | smart_pointer.cpp:65:28:65:46 | call to make_unique | TAINT |
32703272
| smart_pointer.cpp:65:58:65:58 | 0 | smart_pointer.cpp:65:28:65:46 | call to make_unique | TAINT |
3273+
| smart_pointer.cpp:66:10:66:10 | p | smart_pointer.cpp:66:11:66:11 | call to operator-> | |
32713274
| smart_pointer.cpp:66:10:66:10 | ref arg p | smart_pointer.cpp:67:10:67:10 | p | |
3275+
| smart_pointer.cpp:67:10:67:10 | p | smart_pointer.cpp:67:11:67:11 | call to operator-> | |
3276+
| smart_pointer.cpp:76:45:76:45 | p | smart_pointer.cpp:77:3:77:3 | p | |
3277+
| smart_pointer.cpp:76:45:76:45 | p | smart_pointer.cpp:78:8:78:8 | p | |
3278+
| smart_pointer.cpp:76:45:76:45 | p | smart_pointer.cpp:79:8:79:8 | p | |
3279+
| smart_pointer.cpp:76:67:76:67 | q | smart_pointer.cpp:81:3:81:3 | q | |
3280+
| smart_pointer.cpp:76:67:76:67 | q | smart_pointer.cpp:82:8:82:8 | q | |
3281+
| smart_pointer.cpp:76:67:76:67 | q | smart_pointer.cpp:83:8:83:8 | q | |
3282+
| smart_pointer.cpp:76:67:76:67 | q | smart_pointer.cpp:84:8:84:8 | q | |
3283+
| smart_pointer.cpp:77:3:77:3 | p | smart_pointer.cpp:77:4:77:4 | call to operator-> | |
3284+
| smart_pointer.cpp:77:3:77:3 | ref arg p | smart_pointer.cpp:78:8:78:8 | p | |
3285+
| smart_pointer.cpp:77:3:77:3 | ref arg p | smart_pointer.cpp:79:8:79:8 | p | |
3286+
| smart_pointer.cpp:77:3:77:17 | ... = ... | smart_pointer.cpp:77:6:77:6 | x [post update] | |
3287+
| smart_pointer.cpp:77:3:77:17 | ... = ... | smart_pointer.cpp:78:11:78:11 | x | |
3288+
| smart_pointer.cpp:77:4:77:4 | call to operator-> [post update] | smart_pointer.cpp:77:3:77:3 | ref arg p | |
3289+
| smart_pointer.cpp:77:10:77:15 | call to source | smart_pointer.cpp:77:3:77:17 | ... = ... | |
3290+
| smart_pointer.cpp:78:8:78:8 | p | smart_pointer.cpp:78:9:78:9 | call to operator-> | |
3291+
| smart_pointer.cpp:78:8:78:8 | ref arg p | smart_pointer.cpp:79:8:79:8 | p | |
3292+
| smart_pointer.cpp:79:8:79:8 | p | smart_pointer.cpp:79:9:79:9 | call to operator-> | |
3293+
| smart_pointer.cpp:81:3:81:3 | q | smart_pointer.cpp:81:4:81:4 | call to operator-> | |
3294+
| smart_pointer.cpp:81:3:81:3 | ref arg q | smart_pointer.cpp:82:8:82:8 | q | |
3295+
| smart_pointer.cpp:81:3:81:3 | ref arg q | smart_pointer.cpp:83:8:83:8 | q | |
3296+
| smart_pointer.cpp:81:3:81:3 | ref arg q | smart_pointer.cpp:84:8:84:8 | q | |
3297+
| smart_pointer.cpp:81:3:81:20 | ... = ... | smart_pointer.cpp:81:9:81:9 | x [post update] | |
3298+
| smart_pointer.cpp:81:3:81:20 | ... = ... | smart_pointer.cpp:82:14:82:14 | x | |
3299+
| smart_pointer.cpp:81:4:81:4 | call to operator-> [post update] | smart_pointer.cpp:81:3:81:3 | ref arg q | |
3300+
| smart_pointer.cpp:81:13:81:18 | call to source | smart_pointer.cpp:81:3:81:20 | ... = ... | |
3301+
| smart_pointer.cpp:82:8:82:8 | q | smart_pointer.cpp:82:9:82:9 | call to operator-> | |
3302+
| smart_pointer.cpp:82:8:82:8 | ref arg q | smart_pointer.cpp:83:8:83:8 | q | |
3303+
| smart_pointer.cpp:82:8:82:8 | ref arg q | smart_pointer.cpp:84:8:84:8 | q | |
3304+
| smart_pointer.cpp:83:8:83:8 | q | smart_pointer.cpp:83:9:83:9 | call to operator-> | |
3305+
| smart_pointer.cpp:83:8:83:8 | ref arg q | smart_pointer.cpp:84:8:84:8 | q | |
3306+
| smart_pointer.cpp:84:8:84:8 | q | smart_pointer.cpp:84:9:84:9 | call to operator-> | |
3307+
| smart_pointer.cpp:87:17:87:18 | pa | smart_pointer.cpp:88:5:88:6 | pa | |
3308+
| smart_pointer.cpp:88:5:88:20 | ... = ... | smart_pointer.cpp:88:9:88:9 | x [post update] | |
3309+
| smart_pointer.cpp:88:13:88:18 | call to source | smart_pointer.cpp:88:5:88:20 | ... = ... | |
3310+
| smart_pointer.cpp:92:25:92:50 | call to unique_ptr | smart_pointer.cpp:93:11:93:11 | p | |
3311+
| smart_pointer.cpp:92:25:92:50 | call to unique_ptr | smart_pointer.cpp:94:8:94:8 | p | |
3312+
| smart_pointer.cpp:93:11:93:11 | p | smart_pointer.cpp:93:13:93:15 | call to get | |
3313+
| smart_pointer.cpp:93:11:93:11 | ref arg p | smart_pointer.cpp:94:8:94:8 | p | |
3314+
| smart_pointer.cpp:93:13:93:15 | ref arg call to get | smart_pointer.cpp:93:11:93:11 | ref arg p | |
3315+
| smart_pointer.cpp:94:8:94:8 | p | smart_pointer.cpp:94:9:94:9 | call to operator-> | |
32723316
| standalone_iterators.cpp:39:45:39:51 | source1 | standalone_iterators.cpp:39:45:39:51 | source1 | |
32733317
| standalone_iterators.cpp:39:45:39:51 | source1 | standalone_iterators.cpp:40:11:40:17 | source1 | |
32743318
| standalone_iterators.cpp:39:45:39:51 | source1 | standalone_iterators.cpp:41:12:41:18 | source1 | |

cpp/ql/test/library-tests/dataflow/taint-tests/smart_pointer.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,31 @@ void test_shared_field_member() {
6565
std::unique_ptr<A> p = std::make_unique<A>(source(), 0);
6666
sink(p->x); // $ MISSING: ast,ir
6767
sink(p->y); // not tainted
68+
}
69+
70+
struct B {
71+
A a1;
72+
A a2;
73+
int z;
74+
};
75+
76+
void test_operator_arrow(std::unique_ptr<A> p, std::unique_ptr<B> q) {
77+
p->x = source();
78+
sink(p->x); // $ ast MISSING: ir
79+
sink(p->y);
80+
81+
q->a1.x = source();
82+
sink(q->a1.x); // $ ast MISSING: ir
83+
sink(q->a1.y);
84+
sink(q->a2.x);
85+
}
86+
87+
void taint_x(A* pa) {
88+
pa->x = source();
89+
}
90+
91+
void reverse_taint_smart_pointer() {
92+
std::unique_ptr<A> p = std::unique_ptr<A>(new A);
93+
taint_x(p.get());
94+
sink(p->x); // $ ast MISSING: ir
6895
}

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/AssignWhereCompareMeant/AssignWhereCompareMeant.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,7 @@
1919
| test.cpp:144:32:144:36 | ... = ... | Use of '=' where '==' may have been intended. |
2020
| test.cpp:150:32:150:36 | ... = ... | Use of '=' where '==' may have been intended. |
2121
| test.cpp:153:46:153:50 | ... = ... | Use of '=' where '==' may have been intended. |
22+
| test.cpp:166:22:166:27 | ... = ... | Use of '=' where '==' may have been intended. |
23+
| test.cpp:168:24:168:29 | ... = ... | Use of '=' where '==' may have been intended. |
24+
| test.cpp:169:23:169:28 | ... = ... | Use of '=' where '==' may have been intended. |
25+
| test.cpp:171:7:171:12 | ... = ... | Use of '=' where '==' may have been intended. |

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/AssignWhereCompareMeant/test.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,3 +153,21 @@ void f3(int x, int y) {
153153
if((x == 10) || ((z == z) && (x == 1)) && (y = 2)) { // BAD
154154
}
155155
}
156+
157+
bool use(int);
158+
159+
void f4(int x, bool b) {
160+
if((x = 10) && use(x)) {} // GOOD: This is likely just a short-hand way of writing an assignment
161+
// followed by a boolean check.
162+
if((x = 10) && b && use(x)) {} // GOOD: Same reason as above
163+
if((x = 10) && use(x) && b) {} // GOOD: Same reason as above
164+
if((x = 10) && (use(x) && b)) {} // GOOD: Same reason as above
165+
166+
if(use(x) && b && (x = 10)) {} // BAD: The assignment is the last thing that happens in the comparison.
167+
// This doesn't match the usual pattern.
168+
if((use(x) && b) && (x = 10)) {} // BAD: Same reason as above
169+
if(use(x) && (b && (x = 10))) {} // BAD: Same reason as above
170+
171+
if((x = 10) || use(x)) {} // BAD: This doesn't follow the usual style of writing an assignment in
172+
// a boolean check.
173+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* Support for the Dapper ORM library has been added to the SQL injection checks.

0 commit comments

Comments
 (0)