Skip to content

Commit a460e3a

Browse files
committed
Merge branch 'main' into ast-flow-smart-pointers
2 parents 0a6aef7 + 2d618d6 commit a460e3a

File tree

175 files changed

+4013
-538
lines changed

Some content is hidden

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

175 files changed

+4013
-538
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/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ class WrongCheckErrorOperatorNew extends FunctionCall {
7272
}
7373

7474
/**
75-
* Holds if `(std::nothrow)` exists in call `operator new`.
75+
* Holds if `(std::nothrow)` or `(std::noexcept)` exists in call `operator new`.
7676
*/
77-
predicate isExistsNothrow() { this.getAChild().toString() = "nothrow" }
77+
predicate isExistsNothrow() { getTarget().isNoExcept() or getTarget().isNoThrow() }
7878
}
7979

8080
from WrongCheckErrorOperatorNew op

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/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/test.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ using namespace std;
1414

1515
void* operator new(std::size_t _Size);
1616
void* operator new[](std::size_t _Size);
17-
void* operator new( std::size_t count, const std::nothrow_t& tag );
18-
void* operator new[]( std::size_t count, const std::nothrow_t& tag );
17+
void* operator new( std::size_t count, const std::nothrow_t& tag ) noexcept;
18+
void* operator new[]( std::size_t count, const std::nothrow_t& tag ) noexcept;
1919

2020
void badNew_0_0()
2121
{

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

Lines changed: 63 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3266,17 +3266,19 @@
32663266
| smart_pointer.cpp:47:11:47:11 | p | smart_pointer.cpp:47:10:47:10 | call to operator* | TAINT |
32673267
| smart_pointer.cpp:51:30:51:50 | call to make_shared | smart_pointer.cpp:52:10:52:10 | p | |
32683268
| smart_pointer.cpp:51:52:51:57 | call to source | smart_pointer.cpp:51:30:51:50 | call to make_shared | TAINT |
3269-
| smart_pointer.cpp:52:10:52:10 | p | smart_pointer.cpp:52:12:52:14 | call to get | TAINT |
3269+
| smart_pointer.cpp:52:10:52:10 | p | smart_pointer.cpp:52:12:52:14 | call to get | |
3270+
| smart_pointer.cpp:52:12:52:14 | ref arg call to get | smart_pointer.cpp:52:10:52:10 | ref arg p | |
32703271
| smart_pointer.cpp:56:30:56:50 | call to make_unique | smart_pointer.cpp:57:10:57:10 | p | |
32713272
| smart_pointer.cpp:56:52:56:57 | call to source | smart_pointer.cpp:56:30:56:50 | call to make_unique | TAINT |
3272-
| smart_pointer.cpp:57:10:57:10 | p | smart_pointer.cpp:57:12:57:14 | call to get | TAINT |
3273+
| smart_pointer.cpp:57:10:57:10 | p | smart_pointer.cpp:57:12:57:14 | call to get | |
3274+
| smart_pointer.cpp:57:12:57:14 | ref arg call to get | smart_pointer.cpp:57:10:57:10 | ref arg p | |
32733275
| smart_pointer.cpp:65:28:65:46 | call to make_unique | smart_pointer.cpp:66:10:66:10 | p | |
32743276
| smart_pointer.cpp:65:28:65:46 | call to make_unique | smart_pointer.cpp:67:10:67:10 | p | |
32753277
| smart_pointer.cpp:65:48:65:53 | call to source | smart_pointer.cpp:65:28:65:46 | call to make_unique | TAINT |
32763278
| smart_pointer.cpp:65:58:65:58 | 0 | smart_pointer.cpp:65:28:65:46 | call to make_unique | TAINT |
3277-
| smart_pointer.cpp:66:10:66:10 | p | smart_pointer.cpp:66:11:66:11 | call to operator-> | TAINT |
3279+
| smart_pointer.cpp:66:10:66:10 | p | smart_pointer.cpp:66:11:66:11 | call to operator-> | |
32783280
| smart_pointer.cpp:66:10:66:10 | ref arg p | smart_pointer.cpp:67:10:67:10 | p | |
3279-
| smart_pointer.cpp:67:10:67:10 | p | smart_pointer.cpp:67:11:67:11 | call to operator-> | TAINT |
3281+
| smart_pointer.cpp:67:10:67:10 | p | smart_pointer.cpp:67:11:67:11 | call to operator-> | |
32803282
| smart_pointer.cpp:70:37:70:39 | ptr | smart_pointer.cpp:70:37:70:39 | ptr | |
32813283
| smart_pointer.cpp:70:37:70:39 | ptr | smart_pointer.cpp:71:4:71:6 | ptr | |
32823284
| smart_pointer.cpp:71:3:71:3 | call to operator* [post update] | smart_pointer.cpp:70:37:70:39 | ptr | |
@@ -3289,6 +3291,63 @@
32893291
| smart_pointer.cpp:76:13:76:13 | call to shared_ptr [post update] | smart_pointer.cpp:77:9:77:9 | p | |
32903292
| smart_pointer.cpp:76:13:76:13 | p | smart_pointer.cpp:76:13:76:13 | call to shared_ptr | |
32913293
| smart_pointer.cpp:77:9:77:9 | p | smart_pointer.cpp:77:8:77:8 | call to operator* | TAINT |
3294+
| smart_pointer.cpp:86:45:86:45 | p | smart_pointer.cpp:86:45:86:45 | p | |
3295+
| smart_pointer.cpp:86:45:86:45 | p | smart_pointer.cpp:87:3:87:3 | p | |
3296+
| smart_pointer.cpp:86:45:86:45 | p | smart_pointer.cpp:88:8:88:8 | p | |
3297+
| smart_pointer.cpp:86:45:86:45 | p | smart_pointer.cpp:89:8:89:8 | p | |
3298+
| smart_pointer.cpp:86:67:86:67 | q | smart_pointer.cpp:86:67:86:67 | q | |
3299+
| smart_pointer.cpp:86:67:86:67 | q | smart_pointer.cpp:91:3:91:3 | q | |
3300+
| smart_pointer.cpp:86:67:86:67 | q | smart_pointer.cpp:92:8:92:8 | q | |
3301+
| smart_pointer.cpp:86:67:86:67 | q | smart_pointer.cpp:93:8:93:8 | q | |
3302+
| smart_pointer.cpp:86:67:86:67 | q | smart_pointer.cpp:94:8:94:8 | q | |
3303+
| smart_pointer.cpp:87:3:87:3 | p | smart_pointer.cpp:87:4:87:4 | call to operator-> | |
3304+
| smart_pointer.cpp:87:3:87:3 | ref arg p | smart_pointer.cpp:86:45:86:45 | p | |
3305+
| smart_pointer.cpp:87:3:87:3 | ref arg p | smart_pointer.cpp:88:8:88:8 | p | |
3306+
| smart_pointer.cpp:87:3:87:3 | ref arg p | smart_pointer.cpp:89:8:89:8 | p | |
3307+
| smart_pointer.cpp:87:3:87:17 | ... = ... | smart_pointer.cpp:87:6:87:6 | x [post update] | |
3308+
| smart_pointer.cpp:87:3:87:17 | ... = ... | smart_pointer.cpp:88:11:88:11 | x | |
3309+
| smart_pointer.cpp:87:4:87:4 | call to operator-> [post update] | smart_pointer.cpp:86:45:86:45 | p | |
3310+
| smart_pointer.cpp:87:4:87:4 | call to operator-> [post update] | smart_pointer.cpp:87:3:87:3 | ref arg p | |
3311+
| smart_pointer.cpp:87:4:87:4 | call to operator-> [post update] | smart_pointer.cpp:88:8:88:8 | p | |
3312+
| smart_pointer.cpp:87:4:87:4 | call to operator-> [post update] | smart_pointer.cpp:89:8:89:8 | p | |
3313+
| smart_pointer.cpp:87:10:87:15 | call to source | smart_pointer.cpp:87:3:87:17 | ... = ... | |
3314+
| smart_pointer.cpp:88:8:88:8 | p | smart_pointer.cpp:88:9:88:9 | call to operator-> | |
3315+
| smart_pointer.cpp:88:8:88:8 | ref arg p | smart_pointer.cpp:86:45:86:45 | p | |
3316+
| smart_pointer.cpp:88:8:88:8 | ref arg p | smart_pointer.cpp:89:8:89:8 | p | |
3317+
| smart_pointer.cpp:89:8:89:8 | p | smart_pointer.cpp:89:9:89:9 | call to operator-> | |
3318+
| smart_pointer.cpp:89:8:89:8 | ref arg p | smart_pointer.cpp:86:45:86:45 | p | |
3319+
| smart_pointer.cpp:91:3:91:3 | q | smart_pointer.cpp:91:4:91:4 | call to operator-> | |
3320+
| smart_pointer.cpp:91:3:91:3 | ref arg q | smart_pointer.cpp:86:67:86:67 | q | |
3321+
| smart_pointer.cpp:91:3:91:3 | ref arg q | smart_pointer.cpp:92:8:92:8 | q | |
3322+
| smart_pointer.cpp:91:3:91:3 | ref arg q | smart_pointer.cpp:93:8:93:8 | q | |
3323+
| smart_pointer.cpp:91:3:91:3 | ref arg q | smart_pointer.cpp:94:8:94:8 | q | |
3324+
| smart_pointer.cpp:91:3:91:20 | ... = ... | smart_pointer.cpp:91:9:91:9 | x [post update] | |
3325+
| smart_pointer.cpp:91:3:91:20 | ... = ... | smart_pointer.cpp:92:14:92:14 | x | |
3326+
| smart_pointer.cpp:91:4:91:4 | call to operator-> [post update] | smart_pointer.cpp:86:67:86:67 | q | |
3327+
| smart_pointer.cpp:91:4:91:4 | call to operator-> [post update] | smart_pointer.cpp:91:3:91:3 | ref arg q | |
3328+
| smart_pointer.cpp:91:4:91:4 | call to operator-> [post update] | smart_pointer.cpp:92:8:92:8 | q | |
3329+
| smart_pointer.cpp:91:4:91:4 | call to operator-> [post update] | smart_pointer.cpp:93:8:93:8 | q | |
3330+
| smart_pointer.cpp:91:4:91:4 | call to operator-> [post update] | smart_pointer.cpp:94:8:94:8 | q | |
3331+
| smart_pointer.cpp:91:13:91:18 | call to source | smart_pointer.cpp:91:3:91:20 | ... = ... | |
3332+
| smart_pointer.cpp:92:8:92:8 | q | smart_pointer.cpp:92:9:92:9 | call to operator-> | |
3333+
| smart_pointer.cpp:92:8:92:8 | ref arg q | smart_pointer.cpp:86:67:86:67 | q | |
3334+
| smart_pointer.cpp:92:8:92:8 | ref arg q | smart_pointer.cpp:93:8:93:8 | q | |
3335+
| smart_pointer.cpp:92:8:92:8 | ref arg q | smart_pointer.cpp:94:8:94:8 | q | |
3336+
| smart_pointer.cpp:93:8:93:8 | q | smart_pointer.cpp:93:9:93:9 | call to operator-> | |
3337+
| smart_pointer.cpp:93:8:93:8 | ref arg q | smart_pointer.cpp:86:67:86:67 | q | |
3338+
| smart_pointer.cpp:93:8:93:8 | ref arg q | smart_pointer.cpp:94:8:94:8 | q | |
3339+
| smart_pointer.cpp:94:8:94:8 | q | smart_pointer.cpp:94:9:94:9 | call to operator-> | |
3340+
| smart_pointer.cpp:94:8:94:8 | ref arg q | smart_pointer.cpp:86:67:86:67 | q | |
3341+
| smart_pointer.cpp:97:17:97:18 | pa | smart_pointer.cpp:98:5:98:6 | pa | |
3342+
| smart_pointer.cpp:98:5:98:20 | ... = ... | smart_pointer.cpp:98:9:98:9 | x [post update] | |
3343+
| smart_pointer.cpp:98:13:98:18 | call to source | smart_pointer.cpp:98:5:98:20 | ... = ... | |
3344+
| smart_pointer.cpp:102:25:102:50 | call to unique_ptr | smart_pointer.cpp:103:11:103:11 | p | |
3345+
| smart_pointer.cpp:102:25:102:50 | call to unique_ptr | smart_pointer.cpp:104:8:104:8 | p | |
3346+
| smart_pointer.cpp:103:11:103:11 | p | smart_pointer.cpp:103:13:103:15 | call to get | |
3347+
| smart_pointer.cpp:103:11:103:11 | ref arg p | smart_pointer.cpp:104:8:104:8 | p | |
3348+
| smart_pointer.cpp:103:13:103:15 | ref arg call to get | smart_pointer.cpp:103:11:103:11 | ref arg p | |
3349+
| smart_pointer.cpp:103:13:103:15 | ref arg call to get | smart_pointer.cpp:104:8:104:8 | p | |
3350+
| smart_pointer.cpp:104:8:104:8 | p | smart_pointer.cpp:104:9:104:9 | call to operator-> | |
32923351
| standalone_iterators.cpp:39:45:39:51 | source1 | standalone_iterators.cpp:39:45:39:51 | source1 | |
32933352
| standalone_iterators.cpp:39:45:39:51 | source1 | standalone_iterators.cpp:40:11:40:17 | source1 | |
32943353
| 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
@@ -75,4 +75,31 @@ int test_from_issue_5190() {
7575
std::shared_ptr<int> p(new int);
7676
getNumber(p);
7777
sink(*p); // $ ast MISSING: ir
78+
}
79+
80+
struct B {
81+
A a1;
82+
A a2;
83+
int z;
84+
};
85+
86+
void test_operator_arrow(std::unique_ptr<A> p, std::unique_ptr<B> q) {
87+
p->x = source();
88+
sink(p->x); // $ ast MISSING: ir
89+
sink(p->y);
90+
91+
q->a1.x = source();
92+
sink(q->a1.x); // $ ast MISSING: ir
93+
sink(q->a1.y);
94+
sink(q->a2.x);
95+
}
96+
97+
void taint_x(A* pa) {
98+
pa->x = source();
99+
}
100+
101+
void reverse_taint_smart_pointer() {
102+
std::unique_ptr<A> p = std::unique_ptr<A>(new A);
103+
taint_x(p.get());
104+
sink(p->x); // $ ast MISSING: ir
78105
}

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. |

0 commit comments

Comments
 (0)