Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 131 additions & 21 deletions rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,10 @@ module Node {
* Nodes corresponding to AST elements, for example `ExprNode`, usually refer
* to the value before the update.
*/
final class PostUpdateNode extends Node, TArgumentPostUpdateNode {
final class PostUpdateNode extends Node, TExprPostUpdateNode {
private ExprCfgNode n;

PostUpdateNode() { this = TArgumentPostUpdateNode(n) }
PostUpdateNode() { this = TExprPostUpdateNode(n) }

/** Gets the node before the state update. */
Node getPreUpdateNode() { result = TExprNode(n) }
Expand Down Expand Up @@ -449,6 +449,49 @@ private class VariantFieldContent extends VariantContent, TVariantFieldContent {
}
}

/** A canonical path pointing to a struct. */
private class StructCanonicalPath extends MkStructCanonicalPath {
CrateOriginOption crate;
string path;

StructCanonicalPath() { this = MkStructCanonicalPath(crate, path) }

/** Gets the underlying struct. */
Struct getStruct() { hasExtendedCanonicalPath(result, crate, path) }

string toString() { result = this.getStruct().getName().getText() }

Location getLocation() { result = this.getStruct().getLocation() }
}

/** Content stored in a field on a struct. */
private class StructFieldContent extends VariantContent, TStructFieldContent {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should extend Content instead of VariantContent.

private StructCanonicalPath s;
private string field_;

StructFieldContent() { this = TStructFieldContent(s, field_) }

StructCanonicalPath getStructCanonicalPath(string field) { result = s and field = field_ }

override string toString() { result = s.toString() + "." + field_.toString() }
}

/**
* Content stored at a position in a tuple.
*
* NOTE: Unlike `struct`s and `enum`s tuples are structural and not nominal,
* hence we don't store a canonical path for them.
*/
private class TuplePositionContent extends VariantContent, TTuplePositionContent {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

private int pos;

TuplePositionContent() { this = TTuplePositionContent(pos) }

int getPosition() { result = pos }

override string toString() { result = "tuple." + pos.toString() }
}

/** A value that represents a set of `Content`s. */
abstract class ContentSet extends TContentSet {
/** Gets a textual representation of this element. */
Expand Down Expand Up @@ -608,6 +651,14 @@ module RustDataFlow implements InputSig<Location> {
*/
predicate jumpStep(Node node1, Node node2) { none() }

/** Holds if path `p` resolves to struct `s`. */
private predicate pathResolveToStructCanonicalPath(Path p, StructCanonicalPath s) {
exists(CrateOriginOption crate, string path |
resolveExtendedCanonicalPath(p, crate, path) and
s = MkStructCanonicalPath(crate, path)
)
}

/** Holds if path `p` resolves to variant `v`. */
private predicate pathResolveToVariantCanonicalPath(Path p, VariantCanonicalPath v) {
exists(CrateOriginOption crate, string path |
Expand Down Expand Up @@ -636,6 +687,12 @@ module RustDataFlow implements InputSig<Location> {
pathResolveToVariantCanonicalPath(p.getPath(), v)
}

/** Holds if `p` destructs an struct `s`. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a struct

pragma[nomagic]
private predicate structDestruction(RecordPat p, StructCanonicalPath s) {
pathResolveToStructCanonicalPath(p.getPath(), s)
}

/**
* Holds if data can flow from `node1` to `node2` via a read of `c`. Thus,
* `node1` references an object with a content `c.getAReadContent()` whose
Expand All @@ -652,10 +709,24 @@ module RustDataFlow implements InputSig<Location> {
or
exists(RecordPatCfgNode pat, string field |
pat = node1.asPat() and
recordVariantDestruction(pat.getPat(),
c.(VariantFieldContent).getVariantCanonicalPath(field)) and
(
// Pattern destructs a struct-like variant.
recordVariantDestruction(pat.getPat(),
c.(VariantFieldContent).getVariantCanonicalPath(field))
or
// Pattern destructs a struct.
structDestruction(pat.getPat(), c.(StructFieldContent).getStructCanonicalPath(field))
) and
node2.asPat() = pat.getFieldPat(field)
)
or
exists(FieldExprCfgNode access |
// Read of a tuple entry
access.getNameRef().getText().toInt() = c.(TuplePositionContent).getPosition() and
// TODO: Handle read of a struct field.
node1.asExpr() = access.getExpr() and
node2.asExpr() = access
)
)
}

Expand All @@ -671,30 +742,44 @@ module RustDataFlow implements InputSig<Location> {
pathResolveToVariantCanonicalPath(re.getPath(), v)
}

/** Holds if `re` constructs a struct value of type `v`. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v -> s

pragma[nomagic]
private predicate structConstruction(RecordExpr re, StructCanonicalPath s) {
pathResolveToStructCanonicalPath(re.getPath(), s)
}

/**
* Holds if data can flow from `node1` to `node2` via a store into `c`. Thus,
* `node2` references an object with a content `c.getAStoreContent()` that
* contains the value of `node1`.
*/
predicate storeStep(Node node1, ContentSet cs, Node node2) {
exists(Content c | c = cs.(SingletonContentSet).getContent() |
node2.asExpr() =
any(CallExprCfgNode call, int pos |
tupleVariantConstruction(call.getCallExpr(),
c.(VariantPositionContent).getVariantCanonicalPath(pos)) and
node1.asExpr() = call.getArgument(pos)
|
call
)
exists(CallExprCfgNode call, int pos |
tupleVariantConstruction(call.getCallExpr(),
c.(VariantPositionContent).getVariantCanonicalPath(pos)) and
node1.asExpr() = call.getArgument(pos) and
node2.asExpr() = call
)
or
node2.asExpr() =
any(RecordExprCfgNode re, string field |
exists(RecordExprCfgNode re, string field |
(
// Expression is for a struct-like enum variant.
recordVariantConstruction(re.getRecordExpr(),
c.(VariantFieldContent).getVariantCanonicalPath(field)) and
node1.asExpr() = re.getFieldExpr(field)
|
re
)
c.(VariantFieldContent).getVariantCanonicalPath(field))
or
// Expression is for a struct.
structConstruction(re.getRecordExpr(),
c.(StructFieldContent).getStructCanonicalPath(field))
) and
node1.asExpr() = re.getFieldExpr(field) and
node2.asExpr() = re
)
or
exists(TupleExprCfgNode tuple |
node1.asExpr() = tuple.getField(c.(TuplePositionContent).getPosition()) and
node2.asExpr() = tuple
)
)
}

Expand All @@ -703,7 +788,14 @@ module RustDataFlow implements InputSig<Location> {
* any value stored inside `f` is cleared at the pre-update node associated with `x`
* in `x.f = newValue`.
*/
predicate clearsContent(Node n, ContentSet c) { none() }
predicate clearsContent(Node n, ContentSet c) {
exists(AssignmentExprCfgNode assignment, FieldExprCfgNode access |
assignment.getLhs() = access and
n.asExpr() = access.getExpr() and
access.getNameRef().getText().toInt() =
c.(SingletonContentSet).getContent().(TuplePositionContent).getPosition()
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part should also give rise to a store step (but where the toNode is the post-update node of n). That should remove the false negative inside tuple_mutation.

}

/**
* Holds if the value that is being tracked is expected to be stored inside content `c`
Expand Down Expand Up @@ -773,7 +865,9 @@ private module Cached {
TExprNode(ExprCfgNode n) or
TParameterNode(ParamBaseCfgNode p) or
TPatNode(PatCfgNode p) or
TArgumentPostUpdateNode(ExprCfgNode e) { isArgumentForCall(e, _, _) } or
TExprPostUpdateNode(ExprCfgNode e) {
isArgumentForCall(e, _, _) or e = any(FieldExprCfgNode access).getExpr()
} or
TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node)

cached
Expand Down Expand Up @@ -811,6 +905,12 @@ private module Cached {
name = ["Ok", "Err"]
}

cached
newtype TStructCanonicalPath =
MkStructCanonicalPath(CrateOriginOption crate, string path) {
exists(Struct s | hasExtendedCanonicalPath(s, crate, path))
}

cached
newtype TContent =
TVariantPositionContent(VariantCanonicalPath v, int pos) {
Expand All @@ -826,6 +926,16 @@ private module Cached {
} or
TVariantFieldContent(VariantCanonicalPath v, string field) {
field = v.getVariant().getFieldList().(RecordFieldList).getAField().getName().getText()
} or
TTuplePositionContent(int pos) {
pos in [0 .. max([
any(TuplePat pat).getNumberOfFields(),
any(FieldExpr access).getNameRef().getText().toInt()
]
)]
} or
TStructFieldContent(StructCanonicalPath s, string field) {
field = s.getStruct().getFieldList().(RecordFieldList).getAField().getName().getText()
}

cached
Expand Down
45 changes: 43 additions & 2 deletions rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ localStep
| main.rs:109:10:109:10 | a | main.rs:110:10:110:10 | a |
| main.rs:110:10:110:10 | a | main.rs:111:5:111:5 | a |
| main.rs:111:5:111:5 | a | main.rs:112:5:112:5 | a |
| main.rs:111:11:111:11 | 2 | main.rs:111:5:111:7 | a.1 |
| main.rs:111:11:111:20 | source(...) | main.rs:111:5:111:7 | a.0 |
| main.rs:112:5:112:5 | a | main.rs:113:10:113:10 | a |
| main.rs:112:11:112:20 | source(...) | main.rs:112:5:112:7 | a.0 |
| main.rs:112:11:112:11 | 2 | main.rs:112:5:112:7 | a.1 |
| main.rs:113:10:113:10 | a | main.rs:114:10:114:10 | a |
| main.rs:118:9:118:9 | [SSA] a | main.rs:119:14:119:14 | a |
| main.rs:118:9:118:9 | a | main.rs:118:9:118:9 | [SSA] a |
Expand Down Expand Up @@ -329,6 +329,31 @@ localStep
| main.rs:304:22:304:22 | n | main.rs:304:22:304:22 | [SSA] n |
| main.rs:304:29:304:35 | sink(...) | main.rs:302:5:305:5 | match s2 { ... } |
storeStep
| main.rs:94:14:94:22 | source(...) | tuple.0 | main.rs:94:13:94:26 | TupleExpr |
| main.rs:94:25:94:25 | 2 | tuple.1 | main.rs:94:13:94:26 | TupleExpr |
| main.rs:100:14:100:14 | 2 | tuple.0 | main.rs:100:13:100:30 | TupleExpr |
| main.rs:100:17:100:26 | source(...) | tuple.1 | main.rs:100:13:100:30 | TupleExpr |
| main.rs:100:29:100:29 | 2 | tuple.2 | main.rs:100:13:100:30 | TupleExpr |
| main.rs:108:18:108:18 | 2 | tuple.0 | main.rs:108:17:108:31 | TupleExpr |
| main.rs:108:21:108:30 | source(...) | tuple.1 | main.rs:108:17:108:31 | TupleExpr |
| main.rs:118:14:118:14 | 3 | tuple.0 | main.rs:118:13:118:27 | TupleExpr |
| main.rs:118:17:118:26 | source(...) | tuple.1 | main.rs:118:13:118:27 | TupleExpr |
| main.rs:119:14:119:14 | a | tuple.0 | main.rs:119:13:119:18 | TupleExpr |
| main.rs:119:17:119:17 | 3 | tuple.1 | main.rs:119:13:119:18 | TupleExpr |
| main.rs:135:12:135:20 | source(...) | Point.x | main.rs:134:13:137:5 | Point {...} |
| main.rs:136:12:136:12 | 2 | Point.y | main.rs:134:13:137:5 | Point {...} |
| main.rs:144:12:144:20 | source(...) | Point.x | main.rs:143:17:146:5 | Point {...} |
| main.rs:145:12:145:12 | 2 | Point.y | main.rs:143:17:146:5 | Point {...} |
| main.rs:154:12:154:21 | source(...) | Point.x | main.rs:153:13:156:5 | Point {...} |
| main.rs:155:12:155:12 | 2 | Point.y | main.rs:153:13:156:5 | Point {...} |
| main.rs:169:16:172:9 | Point {...} | Point3D.plane | main.rs:168:13:174:5 | Point3D {...} |
| main.rs:170:16:170:16 | 2 | Point.x | main.rs:169:16:172:9 | Point {...} |
| main.rs:171:16:171:25 | source(...) | Point.y | main.rs:169:16:172:9 | Point {...} |
| main.rs:173:12:173:12 | 4 | Point3D.z | main.rs:168:13:174:5 | Point3D {...} |
| main.rs:182:16:185:9 | Point {...} | Point3D.plane | main.rs:181:13:187:5 | Point3D {...} |
| main.rs:183:16:183:16 | 2 | Point.x | main.rs:182:16:185:9 | Point {...} |
| main.rs:184:16:184:25 | source(...) | Point.y | main.rs:182:16:185:9 | Point {...} |
| main.rs:186:12:186:12 | 4 | Point3D.z | main.rs:181:13:187:5 | Point3D {...} |
| main.rs:214:19:214:28 | source(...) | Some | main.rs:214:14:214:29 | Some(...) |
| main.rs:215:19:215:19 | 2 | Some | main.rs:215:14:215:20 | Some(...) |
| main.rs:232:29:232:38 | source(...) | A | main.rs:232:14:232:39 | ...::A(...) |
Expand All @@ -338,6 +363,22 @@ storeStep
| main.rs:312:27:312:27 | 0 | Some | main.rs:312:22:312:28 | Some(...) |
readStep
| main.rs:33:9:33:15 | TupleStructPat | Some | main.rs:33:14:33:14 | _ |
| main.rs:95:10:95:10 | a | tuple.0 | main.rs:95:10:95:12 | a.0 |
| main.rs:96:10:96:10 | a | tuple.1 | main.rs:96:10:96:12 | a.1 |
| main.rs:109:10:109:10 | a | tuple.0 | main.rs:109:10:109:12 | a.0 |
| main.rs:110:10:110:10 | a | tuple.1 | main.rs:110:10:110:12 | a.1 |
| main.rs:111:5:111:5 | a | tuple.0 | main.rs:111:5:111:7 | a.0 |
| main.rs:112:5:112:5 | a | tuple.1 | main.rs:112:5:112:7 | a.1 |
| main.rs:113:10:113:10 | a | tuple.0 | main.rs:113:10:113:12 | a.0 |
| main.rs:114:10:114:10 | a | tuple.1 | main.rs:114:10:114:12 | a.1 |
| main.rs:120:10:120:10 | b | tuple.0 | main.rs:120:10:120:12 | b.0 |
| main.rs:120:10:120:12 | b.0 | tuple.0 | main.rs:120:10:120:14 | ... .0 |
| main.rs:121:10:121:10 | b | tuple.0 | main.rs:121:10:121:12 | b.0 |
| main.rs:121:10:121:12 | b.0 | tuple.1 | main.rs:121:10:121:14 | ... .1 |
| main.rs:122:10:122:10 | b | tuple.1 | main.rs:122:10:122:12 | b.1 |
| main.rs:157:9:157:28 | Point {...} | Point.x | main.rs:157:20:157:20 | a |
| main.rs:157:9:157:28 | Point {...} | Point.y | main.rs:157:26:157:26 | b |
| main.rs:189:9:189:45 | Point3D {...} | Point3D.plane | main.rs:189:26:189:39 | Point {...} |
| main.rs:217:9:217:15 | TupleStructPat | Some | main.rs:217:14:217:14 | n |
| main.rs:221:9:221:15 | TupleStructPat | Some | main.rs:221:14:221:14 | n |
| main.rs:235:9:235:25 | TupleStructPat | A | main.rs:235:24:235:24 | n |
Expand Down
40 changes: 40 additions & 0 deletions rust/ql/test/library-tests/dataflow/local/inline-flow.expected
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,22 @@ edges
| main.rs:31:13:31:21 | source(...) | main.rs:36:10:36:10 | b | provenance | |
| main.rs:45:15:45:23 | source(...) | main.rs:47:10:47:10 | b | provenance | |
| main.rs:53:9:53:17 | source(...) | main.rs:54:10:54:10 | i | provenance | |
| main.rs:94:13:94:26 | TupleExpr [tuple.0] | main.rs:95:10:95:10 | a [tuple.0] | provenance | |
| main.rs:94:14:94:22 | source(...) | main.rs:94:13:94:26 | TupleExpr [tuple.0] | provenance | |
| main.rs:95:10:95:10 | a [tuple.0] | main.rs:95:10:95:12 | a.0 | provenance | |
| main.rs:108:17:108:31 | TupleExpr [tuple.1] | main.rs:110:10:110:10 | a [tuple.1] | provenance | |
| main.rs:108:21:108:30 | source(...) | main.rs:108:17:108:31 | TupleExpr [tuple.1] | provenance | |
| main.rs:110:10:110:10 | a [tuple.1] | main.rs:110:10:110:12 | a.1 | provenance | |
| main.rs:118:13:118:27 | TupleExpr [tuple.1] | main.rs:119:14:119:14 | a [tuple.1] | provenance | |
| main.rs:118:17:118:26 | source(...) | main.rs:118:13:118:27 | TupleExpr [tuple.1] | provenance | |
| main.rs:119:13:119:18 | TupleExpr [tuple.0, tuple.1] | main.rs:121:10:121:10 | b [tuple.0, tuple.1] | provenance | |
| main.rs:119:14:119:14 | a [tuple.1] | main.rs:119:13:119:18 | TupleExpr [tuple.0, tuple.1] | provenance | |
| main.rs:121:10:121:10 | b [tuple.0, tuple.1] | main.rs:121:10:121:12 | b.0 [tuple.1] | provenance | |
| main.rs:121:10:121:12 | b.0 [tuple.1] | main.rs:121:10:121:14 | ... .1 | provenance | |
| main.rs:153:13:156:5 | Point {...} [Point.x] | main.rs:157:9:157:28 | Point {...} [Point.x] | provenance | |
| main.rs:154:12:154:21 | source(...) | main.rs:153:13:156:5 | Point {...} [Point.x] | provenance | |
| main.rs:157:9:157:28 | Point {...} [Point.x] | main.rs:157:20:157:20 | a | provenance | |
| main.rs:157:20:157:20 | a | main.rs:158:10:158:10 | a | provenance | |
| main.rs:214:14:214:29 | Some(...) [Some] | main.rs:217:9:217:15 | TupleStructPat [Some] | provenance | |
| main.rs:214:19:214:28 | source(...) | main.rs:214:14:214:29 | Some(...) [Some] | provenance | |
| main.rs:217:9:217:15 | TupleStructPat [Some] | main.rs:217:14:217:14 | n | provenance | |
Expand Down Expand Up @@ -35,6 +51,26 @@ nodes
| main.rs:47:10:47:10 | b | semmle.label | b |
| main.rs:53:9:53:17 | source(...) | semmle.label | source(...) |
| main.rs:54:10:54:10 | i | semmle.label | i |
| main.rs:94:13:94:26 | TupleExpr [tuple.0] | semmle.label | TupleExpr [tuple.0] |
| main.rs:94:14:94:22 | source(...) | semmle.label | source(...) |
| main.rs:95:10:95:10 | a [tuple.0] | semmle.label | a [tuple.0] |
| main.rs:95:10:95:12 | a.0 | semmle.label | a.0 |
| main.rs:108:17:108:31 | TupleExpr [tuple.1] | semmle.label | TupleExpr [tuple.1] |
| main.rs:108:21:108:30 | source(...) | semmle.label | source(...) |
| main.rs:110:10:110:10 | a [tuple.1] | semmle.label | a [tuple.1] |
| main.rs:110:10:110:12 | a.1 | semmle.label | a.1 |
| main.rs:118:13:118:27 | TupleExpr [tuple.1] | semmle.label | TupleExpr [tuple.1] |
| main.rs:118:17:118:26 | source(...) | semmle.label | source(...) |
| main.rs:119:13:119:18 | TupleExpr [tuple.0, tuple.1] | semmle.label | TupleExpr [tuple.0, tuple.1] |
| main.rs:119:14:119:14 | a [tuple.1] | semmle.label | a [tuple.1] |
| main.rs:121:10:121:10 | b [tuple.0, tuple.1] | semmle.label | b [tuple.0, tuple.1] |
| main.rs:121:10:121:12 | b.0 [tuple.1] | semmle.label | b.0 [tuple.1] |
| main.rs:121:10:121:14 | ... .1 | semmle.label | ... .1 |
| main.rs:153:13:156:5 | Point {...} [Point.x] | semmle.label | Point {...} [Point.x] |
| main.rs:154:12:154:21 | source(...) | semmle.label | source(...) |
| main.rs:157:9:157:28 | Point {...} [Point.x] | semmle.label | Point {...} [Point.x] |
| main.rs:157:20:157:20 | a | semmle.label | a |
| main.rs:158:10:158:10 | a | semmle.label | a |
| main.rs:214:14:214:29 | Some(...) [Some] | semmle.label | Some(...) [Some] |
| main.rs:214:19:214:28 | source(...) | semmle.label | source(...) |
| main.rs:217:9:217:15 | TupleStructPat [Some] | semmle.label | TupleStructPat [Some] |
Expand Down Expand Up @@ -65,6 +101,10 @@ testFailures
| main.rs:36:10:36:10 | b | main.rs:31:13:31:21 | source(...) | main.rs:36:10:36:10 | b | $@ | main.rs:31:13:31:21 | source(...) | source(...) |
| main.rs:47:10:47:10 | b | main.rs:45:15:45:23 | source(...) | main.rs:47:10:47:10 | b | $@ | main.rs:45:15:45:23 | source(...) | source(...) |
| main.rs:54:10:54:10 | i | main.rs:53:9:53:17 | source(...) | main.rs:54:10:54:10 | i | $@ | main.rs:53:9:53:17 | source(...) | source(...) |
| main.rs:95:10:95:12 | a.0 | main.rs:94:14:94:22 | source(...) | main.rs:95:10:95:12 | a.0 | $@ | main.rs:94:14:94:22 | source(...) | source(...) |
| main.rs:110:10:110:12 | a.1 | main.rs:108:21:108:30 | source(...) | main.rs:110:10:110:12 | a.1 | $@ | main.rs:108:21:108:30 | source(...) | source(...) |
| main.rs:121:10:121:14 | ... .1 | main.rs:118:17:118:26 | source(...) | main.rs:121:10:121:14 | ... .1 | $@ | main.rs:118:17:118:26 | source(...) | source(...) |
| main.rs:158:10:158:10 | a | main.rs:154:12:154:21 | source(...) | main.rs:158:10:158:10 | a | $@ | main.rs:154:12:154:21 | source(...) | source(...) |
| main.rs:217:25:217:25 | n | main.rs:214:19:214:28 | source(...) | main.rs:217:25:217:25 | n | $@ | main.rs:214:19:214:28 | source(...) | source(...) |
| main.rs:235:35:235:35 | n | main.rs:232:29:232:38 | source(...) | main.rs:235:35:235:35 | n | $@ | main.rs:232:29:232:38 | source(...) | source(...) |
| main.rs:239:55:239:55 | n | main.rs:232:29:232:38 | source(...) | main.rs:239:55:239:55 | n | $@ | main.rs:232:29:232:38 | source(...) | source(...) |
Expand Down
Loading