Skip to content

Commit a13748f

Browse files
jbjjbj
authored
Merge pull request #2259 from rdmarsh2/rdmarsh/cpp/default-taint-tracking-sources
C++: move sources into DefaultTaintTracking.qll
2 parents 4fffaab + 93ace5b commit a13748f

File tree

10 files changed

+139
-7
lines changed

10 files changed

+139
-7
lines changed

‎cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,32 @@ private predicate predictableInstruction(Instruction instr) {
1919
predictableInstruction(instr.(UnaryInstruction).getUnary())
2020
}
2121

22+
private predicate userInputInstruction(Instruction instr) {
23+
exists(CallInstruction ci, WriteSideEffectInstruction wsei |
24+
userInputArgument(ci.getConvertedResultExpression(), wsei.getIndex()) and
25+
instr = wsei and
26+
wsei.getPrimaryInstruction() = ci
27+
)
28+
or
29+
userInputReturned(instr.getConvertedResultExpression())
30+
or
31+
instr.getConvertedResultExpression() instanceof EnvironmentRead
32+
or
33+
instr
34+
.(LoadInstruction)
35+
.getSourceAddress()
36+
.(VariableAddressInstruction)
37+
.getASTVariable()
38+
.hasName("argv") and
39+
instr.getEnclosingFunction().hasGlobalName("main")
40+
}
41+
2242
private class DefaultTaintTrackingCfg extends DataFlow::Configuration {
2343
DefaultTaintTrackingCfg() { this = "DefaultTaintTrackingCfg" }
2444

25-
override predicate isSource(DataFlow::Node source) { isUserInput(source.asExpr(), _) }
45+
override predicate isSource(DataFlow::Node source) {
46+
userInputInstruction(source.asInstruction())
47+
}
2648

2749
override predicate isSink(DataFlow::Node sink) { any() }
2850

@@ -135,6 +157,8 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) {
135157
// This is part of the translation of `a[i]`, where we want taint to flow
136158
// from `a`.
137159
i2.(PointerAddInstruction).getLeft() = i1
160+
// TODO: robust Chi handling
161+
//
138162
// TODO: Flow from argument to return of known functions: Port missing parts
139163
// of `returnArgument` to the `interfaces.Taint` and `interfaces.DataFlow`
140164
// libraries.
@@ -176,11 +200,30 @@ private Element adjustedSink(DataFlow::Node sink) {
176200
// For compatibility, send flow into a `NotExpr` even if it's part of a
177201
// short-circuiting condition and thus might get skipped.
178202
result.(NotExpr).getOperand() = sink.asExpr()
203+
or
204+
// For compatibility, send flow from argument read side effects to their
205+
// corresponding argument expression
206+
exists(IndirectReadSideEffectInstruction read |
207+
read.getAnOperand().(SideEffectOperand).getAnyDef() = sink.asInstruction() and
208+
read.getArgumentDef().getUnconvertedResultExpression() = result
209+
)
210+
or
211+
exists(BufferReadSideEffectInstruction read |
212+
read.getAnOperand().(SideEffectOperand).getAnyDef() = sink.asInstruction() and
213+
read.getArgumentDef().getUnconvertedResultExpression() = result
214+
)
215+
or
216+
exists(SizedBufferReadSideEffectInstruction read |
217+
read.getAnOperand().(SideEffectOperand).getAnyDef() = sink.asInstruction() and
218+
read.getArgumentDef().getUnconvertedResultExpression() = result
219+
)
179220
}
180221

181222
predicate tainted(Expr source, Element tainted) {
182223
exists(DefaultTaintTrackingCfg cfg, DataFlow::Node sink |
183224
cfg.hasFlow(DataFlow::exprNode(source), sink)
225+
or
226+
cfg.hasFlow(DataFlow::definitionByReferenceNode(source), sink)
184227
|
185228
tainted = adjustedSink(sink)
186229
)

‎cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,48 @@ abstract class PostUpdateNode extends Node {
173173
abstract Node getPreUpdateNode();
174174
}
175175

176+
/**
177+
* A node that represents the value of a variable after a function call that
178+
* may have changed the variable because it's passed by reference.
179+
*
180+
* A typical example would be a call `f(&x)`. Firstly, there will be flow into
181+
* `x` from previous definitions of `x`. Secondly, there will be a
182+
* `DefinitionByReferenceNode` to represent the value of `x` after the call has
183+
* returned. This node will have its `getArgument()` equal to `&x` and its
184+
* `getVariableAccess()` equal to `x`.
185+
*/
186+
class DefinitionByReferenceNode extends Node {
187+
override WriteSideEffectInstruction instr;
188+
189+
/** Gets the argument corresponding to this node. */
190+
Expr getArgument() {
191+
result = instr
192+
.getPrimaryInstruction()
193+
.(CallInstruction)
194+
.getPositionalArgument(instr.getIndex())
195+
.getUnconvertedResultExpression()
196+
or
197+
result = instr
198+
.getPrimaryInstruction()
199+
.(CallInstruction)
200+
.getThisArgument()
201+
.getUnconvertedResultExpression() and
202+
instr.getIndex() = -1
203+
}
204+
205+
/** Gets the parameter through which this value is assigned. */
206+
Parameter getParameter() {
207+
exists(CallInstruction ci | result = ci.getStaticCallTarget().getParameter(instr.getIndex()))
208+
}
209+
}
210+
176211
/**
177212
* Gets the node corresponding to `instr`.
178213
*/
179214
Node instructionNode(Instruction instr) { result.asInstruction() = instr }
180215

216+
DefinitionByReferenceNode definitionByReferenceNode(Expr e) { result.getArgument() = e }
217+
181218
/**
182219
* Gets a `Node` corresponding to `e` or any of its conversions. There is no
183220
* result if `e` is a `Conversion`.

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1236,6 +1236,8 @@ class IndirectReadSideEffectInstruction extends SideEffectInstruction {
12361236
IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect }
12371237

12381238
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
1239+
1240+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12391241
}
12401242

12411243
/**
@@ -1245,6 +1247,8 @@ class BufferReadSideEffectInstruction extends SideEffectInstruction {
12451247
BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect }
12461248

12471249
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
1250+
1251+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12481252
}
12491253

12501254
/**
@@ -1258,12 +1262,14 @@ class SizedBufferReadSideEffectInstruction extends SideEffectInstruction {
12581262
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
12591263

12601264
Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() }
1265+
1266+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12611267
}
12621268

12631269
/**
12641270
* An instruction representing a side effect of a function call.
12651271
*/
1266-
class WriteSideEffectInstruction extends SideEffectInstruction {
1272+
class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction {
12671273
WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode }
12681274

12691275
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1236,6 +1236,8 @@ class IndirectReadSideEffectInstruction extends SideEffectInstruction {
12361236
IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect }
12371237

12381238
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
1239+
1240+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12391241
}
12401242

12411243
/**
@@ -1245,6 +1247,8 @@ class BufferReadSideEffectInstruction extends SideEffectInstruction {
12451247
BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect }
12461248

12471249
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
1250+
1251+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12481252
}
12491253

12501254
/**
@@ -1258,12 +1262,14 @@ class SizedBufferReadSideEffectInstruction extends SideEffectInstruction {
12581262
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
12591263

12601264
Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() }
1265+
1266+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12611267
}
12621268

12631269
/**
12641270
* An instruction representing a side effect of a function call.
12651271
*/
1266-
class WriteSideEffectInstruction extends SideEffectInstruction {
1272+
class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction {
12671273
WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode }
12681274

12691275
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1236,6 +1236,8 @@ class IndirectReadSideEffectInstruction extends SideEffectInstruction {
12361236
IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect }
12371237

12381238
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
1239+
1240+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12391241
}
12401242

12411243
/**
@@ -1245,6 +1247,8 @@ class BufferReadSideEffectInstruction extends SideEffectInstruction {
12451247
BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect }
12461248

12471249
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
1250+
1251+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12481252
}
12491253

12501254
/**
@@ -1258,12 +1262,14 @@ class SizedBufferReadSideEffectInstruction extends SideEffectInstruction {
12581262
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
12591263

12601264
Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() }
1265+
1266+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12611267
}
12621268

12631269
/**
12641270
* An instruction representing a side effect of a function call.
12651271
*/
1266-
class WriteSideEffectInstruction extends SideEffectInstruction {
1272+
class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction {
12671273
WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode }
12681274

12691275
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }

‎cpp/ql/src/semmle/code/cpp/models/Models.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
private import implementations.Fread
12
private import implementations.IdentityFunction
23
private import implementations.Inet
34
private import implementations.Memcpy

‎cpp/ql/src/semmle/code/cpp/models/implementations/Fread.qll‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import semmle.code.cpp.models.interfaces.Alias
2+
3+
class Fread extends AliasFunction {
4+
Fread() { this.hasGlobalName("fread") }
5+
6+
override predicate parameterNeverEscapes(int n) {
7+
n = 0 or
8+
n = 3
9+
}
10+
11+
override predicate parameterEscapesOnlyViaReturn(int n) { none() }
12+
13+
override predicate parameterIsAlwaysReturned(int n) { none() }
14+
}

‎cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import semmle.code.cpp.models.interfaces.FormattingFunction
2+
import semmle.code.cpp.models.interfaces.Alias
23

34
/**
45
* The standard functions `printf`, `wprintf` and their glib variants.
56
*/
6-
class Printf extends FormattingFunction {
7+
class Printf extends FormattingFunction, AliasFunction {
78
Printf() {
89
this instanceof TopLevelFunction and
910
(
@@ -22,6 +23,12 @@ class Printf extends FormattingFunction {
2223
hasGlobalOrStdName("wprintf") or
2324
hasGlobalName("wprintf_s")
2425
}
26+
27+
override predicate parameterNeverEscapes(int n) { n = 0 }
28+
29+
override predicate parameterEscapesOnlyViaReturn(int n) { none() }
30+
31+
override predicate parameterIsAlwaysReturned(int n) { none() }
2532
}
2633

2734
/**

‎csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1236,6 +1236,8 @@ class IndirectReadSideEffectInstruction extends SideEffectInstruction {
12361236
IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect }
12371237

12381238
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
1239+
1240+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12391241
}
12401242

12411243
/**
@@ -1245,6 +1247,8 @@ class BufferReadSideEffectInstruction extends SideEffectInstruction {
12451247
BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect }
12461248

12471249
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
1250+
1251+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12481252
}
12491253

12501254
/**
@@ -1258,12 +1262,14 @@ class SizedBufferReadSideEffectInstruction extends SideEffectInstruction {
12581262
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
12591263

12601264
Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() }
1265+
1266+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12611267
}
12621268

12631269
/**
12641270
* An instruction representing a side effect of a function call.
12651271
*/
1266-
class WriteSideEffectInstruction extends SideEffectInstruction {
1272+
class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction {
12671273
WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode }
12681274

12691275
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }

‎csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1236,6 +1236,8 @@ class IndirectReadSideEffectInstruction extends SideEffectInstruction {
12361236
IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect }
12371237

12381238
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
1239+
1240+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12391241
}
12401242

12411243
/**
@@ -1245,6 +1247,8 @@ class BufferReadSideEffectInstruction extends SideEffectInstruction {
12451247
BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect }
12461248

12471249
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
1250+
1251+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12481252
}
12491253

12501254
/**
@@ -1258,12 +1262,14 @@ class SizedBufferReadSideEffectInstruction extends SideEffectInstruction {
12581262
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }
12591263

12601264
Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() }
1265+
1266+
Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() }
12611267
}
12621268

12631269
/**
12641270
* An instruction representing a side effect of a function call.
12651271
*/
1266-
class WriteSideEffectInstruction extends SideEffectInstruction {
1272+
class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction {
12671273
WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode }
12681274

12691275
Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() }

0 commit comments

Comments
 (0)