Skip to content

Commit 7e20829

Browse files
hvitvedhvitved
committed
Merge remote-tracking branch 'upstream/main' into csharp/rework-summaries
2 parents 6a3859f + 33c990f commit 7e20829

File tree

185 files changed

+3188
-1366
lines changed

Some content is hidden

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

185 files changed

+3188
-1366
lines changed

‎.github/workflows/docs-review.yml

Lines changed: 0 additions & 29 deletions
This file was deleted.

‎config/identical-files.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -429,10 +429,11 @@
429429
"SSA C#": [
430430
"csharp/ql/src/semmle/code/csharp/dataflow/internal/SsaImplCommon.qll",
431431
"csharp/ql/src/semmle/code/csharp/controlflow/internal/pressa/SsaImplCommon.qll",
432-
"csharp/ql/src/semmle/code/csharp/dataflow/internal/basessa/SsaImplCommon.qll"
432+
"csharp/ql/src/semmle/code/csharp/dataflow/internal/basessa/SsaImplCommon.qll",
433+
"csharp/ql/src/semmle/code/cil/internal/SsaImplCommon.qll"
433434
],
434435
"CryptoAlgorithms Python/JS": [
435436
"javascript/ql/src/semmle/javascript/security/CryptoAlgorithms.qll",
436437
"python/ql/src/semmle/crypto/Crypto.qll"
437438
]
438-
}
439+
}

‎cpp/ql/test/query-tests/Security/CWE/CWE-114/semmle/UncontrolledProcessOperation/UncontrolledProcessOperation.expected

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,14 @@ edges
3535
| test.cpp:76:12:76:17 | fgets output argument | test.cpp:78:10:78:15 | buffer |
3636
| test.cpp:76:12:76:17 | fgets output argument | test.cpp:79:10:79:13 | (const char *)... |
3737
| test.cpp:76:12:76:17 | fgets output argument | test.cpp:79:10:79:13 | data |
38+
| test.cpp:98:17:98:22 | buffer | test.cpp:99:15:99:20 | (const char *)... |
39+
| test.cpp:98:17:98:22 | buffer | test.cpp:99:15:99:20 | buffer |
40+
| test.cpp:98:17:98:22 | recv output argument | test.cpp:99:15:99:20 | (const char *)... |
41+
| test.cpp:98:17:98:22 | recv output argument | test.cpp:99:15:99:20 | buffer |
42+
| test.cpp:106:17:106:22 | buffer | test.cpp:107:15:107:20 | (const char *)... |
43+
| test.cpp:106:17:106:22 | buffer | test.cpp:107:15:107:20 | buffer |
44+
| test.cpp:106:17:106:22 | recv output argument | test.cpp:107:15:107:20 | (const char *)... |
45+
| test.cpp:106:17:106:22 | recv output argument | test.cpp:107:15:107:20 | buffer |
3846
nodes
3947
| test.cpp:24:30:24:36 | *command | semmle.label | *command |
4048
| test.cpp:24:30:24:36 | command | semmle.label | command |
@@ -70,10 +78,22 @@ nodes
7078
| test.cpp:79:10:79:13 | (const char *)... | semmle.label | (const char *)... |
7179
| test.cpp:79:10:79:13 | (const char *)... | semmle.label | (const char *)... |
7280
| test.cpp:79:10:79:13 | data | semmle.label | data |
81+
| test.cpp:98:17:98:22 | buffer | semmle.label | buffer |
82+
| test.cpp:98:17:98:22 | recv output argument | semmle.label | recv output argument |
83+
| test.cpp:99:15:99:20 | (const char *)... | semmle.label | (const char *)... |
84+
| test.cpp:99:15:99:20 | (const char *)... | semmle.label | (const char *)... |
85+
| test.cpp:99:15:99:20 | buffer | semmle.label | buffer |
86+
| test.cpp:106:17:106:22 | buffer | semmle.label | buffer |
87+
| test.cpp:106:17:106:22 | recv output argument | semmle.label | recv output argument |
88+
| test.cpp:107:15:107:20 | (const char *)... | semmle.label | (const char *)... |
89+
| test.cpp:107:15:107:20 | (const char *)... | semmle.label | (const char *)... |
90+
| test.cpp:107:15:107:20 | buffer | semmle.label | buffer |
7391
#select
7492
| test.cpp:26:10:26:16 | command | test.cpp:42:18:42:23 | call to getenv | test.cpp:26:10:26:16 | command | The value of this argument may come from $@ and is being passed to system | test.cpp:42:18:42:23 | call to getenv | call to getenv |
7593
| test.cpp:31:10:31:16 | command | test.cpp:43:18:43:23 | call to getenv | test.cpp:31:10:31:16 | command | The value of this argument may come from $@ and is being passed to system | test.cpp:43:18:43:23 | call to getenv | call to getenv |
7694
| test.cpp:62:10:62:15 | buffer | test.cpp:56:12:56:17 | buffer | test.cpp:62:10:62:15 | buffer | The value of this argument may come from $@ and is being passed to system | test.cpp:56:12:56:17 | buffer | buffer |
7795
| test.cpp:63:10:63:13 | data | test.cpp:56:12:56:17 | buffer | test.cpp:63:10:63:13 | data | The value of this argument may come from $@ and is being passed to system | test.cpp:56:12:56:17 | buffer | buffer |
7896
| test.cpp:78:10:78:15 | buffer | test.cpp:76:12:76:17 | buffer | test.cpp:78:10:78:15 | buffer | The value of this argument may come from $@ and is being passed to system | test.cpp:76:12:76:17 | buffer | buffer |
7997
| test.cpp:79:10:79:13 | data | test.cpp:76:12:76:17 | buffer | test.cpp:79:10:79:13 | data | The value of this argument may come from $@ and is being passed to system | test.cpp:76:12:76:17 | buffer | buffer |
98+
| test.cpp:99:15:99:20 | buffer | test.cpp:98:17:98:22 | buffer | test.cpp:99:15:99:20 | buffer | The value of this argument may come from $@ and is being passed to LoadLibrary | test.cpp:98:17:98:22 | buffer | buffer |
99+
| test.cpp:107:15:107:20 | buffer | test.cpp:106:17:106:22 | buffer | test.cpp:107:15:107:20 | buffer | The value of this argument may come from $@ and is being passed to LoadLibrary | test.cpp:106:17:106:22 | buffer | buffer |

‎cpp/ql/test/query-tests/Security/CWE/CWE-114/semmle/UncontrolledProcessOperation/test.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,29 @@ void testReferencePointer2()
8181
system(data2); // BAD [NOT DETECTED]
8282
}
8383
}
84+
85+
// ---
86+
87+
typedef unsigned long size_t;
88+
89+
void accept(int arg, char *buf, size_t *bufSize);
90+
void recv(int arg, char *buf, size_t bufSize);
91+
void LoadLibrary(const char *arg);
92+
93+
void testAcceptRecv(int socket1, int socket2)
94+
{
95+
{
96+
char buffer[1024];
97+
98+
recv(socket1, buffer, 1024);
99+
LoadLibrary(buffer); // BAD: using data from recv
100+
}
101+
102+
{
103+
char buffer[1024];
104+
105+
accept(socket2, 0, 0);
106+
recv(socket2, buffer, 1024);
107+
LoadLibrary(buffer); // BAD: using data from recv
108+
}
109+
}

‎csharp/change-notes/2021-03-24-cil-ssa.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* A static single assignment (SSA) library has been added to the CIL analysis library. The SSA library replaces the existing `DefUse` module, which has been deprecated.

‎csharp/ql/src/semmle/code/cil/BasicBlock.qll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,9 @@ class BasicBlock extends Cached::TBasicBlockStart {
240240

241241
/** Gets a textual representation of this basic block. */
242242
string toString() { result = getFirstNode().toString() }
243+
244+
/** Gets the location of this basic block. */
245+
Location getLocation() { result = this.getFirstNode().getLocation() }
243246
}
244247

245248
/**
@@ -305,7 +308,7 @@ class EntryBasicBlock extends BasicBlock {
305308
}
306309

307310
/** Holds if `bb` is an entry basic block. */
308-
private predicate entryBB(BasicBlock bb) { bb.getFirstNode() instanceof EntryPoint }
311+
private predicate entryBB(BasicBlock bb) { bb.getFirstNode() instanceof MethodImplementation }
309312

310313
/**
311314
* An exit basic block, that is, a basic block whose last node is

‎csharp/ql/src/semmle/code/cil/CIL.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,4 @@ import Attribute
2020
import Stubs
2121
import CustomModifierReceiver
2222
import Parameterizable
23+
import semmle.code.cil.Ssa

‎csharp/ql/src/semmle/code/cil/CallableReturns.qll

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,11 @@ private predicate alwaysNullExpr(Expr expr) {
4242
or
4343
alwaysNullMethod(expr.(StaticCall).getTarget())
4444
or
45-
forex(VariableUpdate vu | DefUse::variableUpdateUse(_, vu, expr) | alwaysNullVariableUpdate(vu))
45+
forex(Ssa::Definition def |
46+
expr = any(Ssa::Definition def0 | def = def0.getAnUltimateDefinition()).getARead()
47+
|
48+
alwaysNullVariableUpdate(def.getVariableUpdate())
49+
)
4650
}
4751

4852
pragma[noinline]
@@ -58,7 +62,9 @@ private predicate alwaysNotNullExpr(Expr expr) {
5862
or
5963
alwaysNotNullMethod(expr.(StaticCall).getTarget())
6064
or
61-
forex(VariableUpdate vu | DefUse::variableUpdateUse(_, vu, expr) |
62-
alwaysNotNullVariableUpdate(vu)
65+
forex(Ssa::Definition def |
66+
expr = any(Ssa::Definition def0 | def = def0.getAnUltimateDefinition()).getARead()
67+
|
68+
alwaysNotNullVariableUpdate(def.getVariableUpdate())
6369
)
6470
}

‎csharp/ql/src/semmle/code/cil/ControlFlow.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ class ControlFlowNode extends @cil_controlflow_node {
99
/** Gets a textual representation of this control flow node. */
1010
string toString() { none() }
1111

12+
/** Gets the location of this control flow node. */
13+
Location getLocation() { none() }
14+
1215
/**
1316
* Gets the number of items this node pushes onto the stack.
1417
* This value is either 0 or 1, except for the instruction `dup`

‎csharp/ql/src/semmle/code/cil/DataFlow.qll

Lines changed: 59 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,40 @@ class Untainted extends TaintType, TExactValue { }
5757
/** A taint type where the data is tainted. */
5858
class Tainted extends TaintType, TTaintedValue { }
5959

60+
private predicate localFlowPhiInput(DataFlowNode input, Ssa::PhiNode phi) {
61+
exists(Ssa::Definition def, BasicBlock bb, int i | phi.hasLastInputRef(def, bb, i) |
62+
def.definesAt(_, bb, i) and
63+
input = def.getVariableUpdate().getSource()
64+
or
65+
input =
66+
any(ReadAccess ra |
67+
bb.getNode(i) = ra and
68+
ra.getTarget() = def.getSourceVariable()
69+
)
70+
)
71+
or
72+
exists(Ssa::PhiNode mid, BasicBlock bb, int i |
73+
localFlowPhiInput(input, mid) and
74+
phi.hasLastInputRef(mid, bb, i) and
75+
mid.definesAt(_, bb, i)
76+
)
77+
}
78+
6079
private predicate localExactStep(DataFlowNode src, DataFlowNode sink) {
6180
src = sink.(Opcodes::Dup).getAnOperand()
6281
or
63-
defUse(_, src, sink)
82+
exists(Ssa::Definition def, VariableUpdate vu |
83+
vu = def.getVariableUpdate() and
84+
src = vu.getSource() and
85+
sink = def.getAFirstRead()
86+
)
87+
or
88+
any(Ssa::Definition def).hasAdjacentReads(src, sink)
6489
or
65-
src = sink.(ParameterReadAccess).getTarget()
90+
exists(Ssa::PhiNode phi |
91+
localFlowPhiInput(src, phi) and
92+
sink = phi.getAFirstRead()
93+
)
6694
or
6795
src = sink.(Conversion).getExpr()
6896
or
@@ -73,12 +101,6 @@ private predicate localExactStep(DataFlowNode src, DataFlowNode sink) {
73101
src = sink.(Return).getExpr()
74102
or
75103
src = sink.(ConditionalBranch).getAnOperand()
76-
or
77-
src = sink.(MethodParameter).getAWrite()
78-
or
79-
exists(VariableUpdate update |
80-
update.getVariable().(Parameter) = sink and src = update.getSource()
81-
)
82104
}
83105

84106
private predicate localTaintStep(DataFlowNode src, DataFlowNode sink) {
@@ -87,8 +109,7 @@ private predicate localTaintStep(DataFlowNode src, DataFlowNode sink) {
87109
src = sink.(UnaryBitwiseOperation).getOperand()
88110
}
89111

90-
cached
91-
module DefUse {
112+
deprecated module DefUse {
92113
/**
93114
* A classification of variable references into reads and writes.
94115
*/
@@ -189,7 +210,7 @@ module DefUse {
189210

190211
/** Holds if the variable update `vu` can be used at the read `use`. */
191212
cached
192-
predicate variableUpdateUse(StackVariable target, VariableUpdate vu, ReadAccess use) {
213+
deprecated predicate variableUpdateUse(StackVariable target, VariableUpdate vu, ReadAccess use) {
193214
defReachesReadWithinBlock(target, vu, use)
194215
or
195216
exists(BasicBlock bb, int i |
@@ -202,23 +223,40 @@ module DefUse {
202223

203224
/** Holds if the update `def` can be used at the read `use`. */
204225
cached
205-
predicate defUse(StackVariable target, Expr def, ReadAccess use) {
226+
deprecated predicate defUse(StackVariable target, Expr def, ReadAccess use) {
206227
exists(VariableUpdate vu | def = vu.getSource() | variableUpdateUse(target, vu, use))
207228
}
208229
}
209230

210-
private import DefUse
211-
212-
abstract library class VariableUpdate extends Instruction {
213-
abstract Expr getSource();
231+
/** A node that updates a variable. */
232+
abstract class VariableUpdate extends DataFlowNode {
233+
/** Gets the value assigned, if any. */
234+
abstract DataFlowNode getSource();
214235

236+
/** Gets the variable that is updated. */
215237
abstract Variable getVariable();
238+
239+
/** Holds if this variable update happens at index `i` in basic block `bb`. */
240+
abstract predicate updatesAt(BasicBlock bb, int i);
241+
}
242+
243+
private class MethodParameterDef extends VariableUpdate, MethodParameter {
244+
override MethodParameter getSource() { result = this }
245+
246+
override MethodParameter getVariable() { result = this }
247+
248+
override predicate updatesAt(BasicBlock bb, int i) {
249+
bb.(EntryBasicBlock).getANode().getImplementation().getMethod() = this.getMethod() and
250+
i = -1
251+
}
216252
}
217253

218254
private class VariableWrite extends VariableUpdate, WriteAccess {
219-
override Expr getSource() { result = getExpr() }
255+
override Expr getSource() { result = this.getExpr() }
220256

221-
override Variable getVariable() { result = getTarget() }
257+
override Variable getVariable() { result = this.getTarget() }
258+
259+
override predicate updatesAt(BasicBlock bb, int i) { this = bb.getNode(i) }
222260
}
223261

224262
private class MethodOutOrRefTarget extends VariableUpdate, Call {
@@ -230,5 +268,7 @@ private class MethodOutOrRefTarget extends VariableUpdate, Call {
230268
result = this.getRawArgument(parameterIndex).(ReadAccess).getTarget()
231269
}
232270

233-
override Expr getSource() { result = this }
271+
override Expr getSource() { none() }
272+
273+
override predicate updatesAt(BasicBlock bb, int i) { this = bb.getNode(i) }
234274
}

0 commit comments

Comments
 (0)