Skip to content

Commit

Permalink
Merge pull request #18291 from paldepind/rust-data-flow-models
Browse files Browse the repository at this point in the history
Rust: Data flow improvements to unlock flow in sqlx test
  • Loading branch information
paldepind authored Dec 18, 2024
2 parents ef2215d + 049fab4 commit 87b9e60
Show file tree
Hide file tree
Showing 21 changed files with 925 additions and 591 deletions.
24 changes: 22 additions & 2 deletions rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,11 @@ private class CapturedVariableContent extends Content, TCapturedVariableContent
override string toString() { result = "captured " + v }
}

/** A value referred to by a reference. */
final class ReferenceContent extends Content, TReferenceContent {
override string toString() { result = "&ref" }
}

/**
* An element in an array.
*/
Expand Down Expand Up @@ -1051,6 +1056,13 @@ module RustDataFlow implements InputSig<Location> {
["crate::option::Option::Some", "crate::result::Result::Ok"]
)
or
exists(PrefixExprCfgNode deref |
c instanceof ReferenceContent and
deref.getOperatorName() = "*" and
node1.asExpr() = deref.getExpr() and
node2.asExpr() = deref
)
or
VariableCapture::readStep(node1, c, node2)
)
or
Expand Down Expand Up @@ -1128,6 +1140,12 @@ module RustDataFlow implements InputSig<Location> {
node2.(PostUpdateNode).getPreUpdateNode().asExpr() = index.getBase()
)
or
exists(RefExprCfgNode ref |
c instanceof ReferenceContent and
node1.asExpr() = ref.getExpr() and
node2.asExpr() = ref
)
or
VariableCapture::storeStep(node1, c, node2)
}

Expand Down Expand Up @@ -1395,7 +1413,8 @@ private module Cached {
e =
[
any(IndexExprCfgNode i).getBase(), any(FieldExprCfgNode access).getExpr(),
any(TryExprCfgNode try).getExpr()
any(TryExprCfgNode try).getExpr(),
any(PrefixExprCfgNode pe | pe.getOperatorName() = "*").getExpr()
]
} or
TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) or
Expand Down Expand Up @@ -1495,7 +1514,8 @@ private module Cached {
TStructFieldContent(StructCanonicalPath s, string field) {
field = s.getStruct().getFieldList().(RecordFieldList).getAField().getName().getText()
} or
TCapturedVariableContent(VariableCapture::CapturedVariable v)
TCapturedVariableContent(VariableCapture::CapturedVariable v) or
TReferenceContent()

cached
newtype TContentSet = TSingletonContentSet(Content c)
Expand Down
18 changes: 7 additions & 11 deletions rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -88,22 +88,16 @@ module SsaInput implements SsaImplCommon::InputSig<Location> {
|
va instanceof VariableReadAccess
or
// For immutable variables, we model a read when they are borrowed
// (although the actual read happens later, if at all).
va = any(RefExpr re).getExpr()
or
// Although compound assignments, like `x += y`, may in fact not read `x`,
// it makes sense to treat them as such
va = any(CompoundAssignmentExpr cae).getLhs()
) and
certain = true
or
// For immutable variables, we model a read when they are borrowed (although the
// actual read happens later, if at all). This only affects the SSA liveness
// analysis.
exists(VariableAccess va |
va = any(RefExpr re).getExpr() and
va = bb.getNode(i).getAstNode() and
v = va.getVariable() and
certain = false
)
or
capturedCallRead(_, bb, i, v) and certain = false
or
capturedExitRead(bb, i, v) and certain = false
Expand Down Expand Up @@ -146,7 +140,9 @@ private predicate adjacentDefReadExt(

/** Holds if `v` is read at index `i` in basic block `bb`. */
private predicate variableReadActual(BasicBlock bb, int i, Variable v) {
exists(VariableReadAccess read |
exists(VariableAccess read |
read instanceof VariableReadAccess or read = any(RefExpr re).getExpr()
|
read.getVariable() = v and
read = bb.getNode(i).getAstNode()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@ module RustTaintTracking implements InputSig<Location, RustDataFlow> {
bindingset[node]
predicate defaultImplicitTaintRead(Node::Node node, ContentSet cs) {
exists(node) and
cs.(SingletonContentSet).getContent() instanceof ArrayElementContent
exists(Content c | c = cs.(SingletonContentSet).getContent() |
c instanceof ArrayElementContent or
c instanceof ReferenceContent
)
}

/**
Expand Down
6 changes: 6 additions & 0 deletions rust/ql/lib/codeql/rust/frameworks/reqwest.model.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
extensions:
- addsTo:
pack: codeql/rust-all
extensible: summaryModel
data:
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::blocking::response::Response>::text", "Argument[self]", "ReturnValue.Variant[crate::result::Result::Ok(0)]", "taint", "manual"]
9 changes: 9 additions & 0 deletions rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,13 @@ extensions:
pack: codeql/rust-all
extensible: summaryModel
data:
# Option
- ["lang:core", "<crate::option::Option>::unwrap", "Argument[self].Variant[crate::option::Option::Some(0)]", "ReturnValue", "value", "manual"]
- ["lang:core", "<crate::option::Option>::unwrap_or", "Argument[self].Variant[crate::option::Option::Some(0)]", "ReturnValue", "value", "manual"]
- ["lang:core", "<crate::option::Option>::unwrap_or", "Argument[0]", "ReturnValue", "value", "manual"]
# Result
- ["lang:core", "<crate::result::Result>::unwrap", "Argument[self].Variant[crate::result::Result::Ok(0)]", "ReturnValue", "value", "manual"]
- ["lang:core", "<crate::result::Result>::unwrap_or", "Argument[self].Variant[crate::result::Result::Ok(0)]", "ReturnValue", "value", "manual"]
- ["lang:core", "<crate::result::Result>::unwrap_or", "Argument[0]", "ReturnValue", "value", "manual"]
# String
- ["lang:alloc", "<crate::string::String>::as_str", "Argument[self]", "ReturnValue", "taint", "manual"]
8 changes: 6 additions & 2 deletions rust/ql/lib/utils/test/InlineFlowTest.qll
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@ private import codeql.rust.dataflow.internal.TaintTrackingImpl
private import codeql.rust.dataflow.internal.ModelsAsData as MaD
private import internal.InlineExpectationsTestImpl as InlineExpectationsTestImpl

// Holds if the target expression of `call` is a path and the string representation of the path is `name`.
/**
* Holds if the target expression of `call` is a path and the string
* representation of the path has `name` as a prefix.
*/
bindingset[name]
private predicate callTargetName(CallExprCfgNode call, string name) {
call.getFunction().(PathExprCfgNode).toString() = name
call.getFunction().(PathExprCfgNode).toString().matches(name + "%")
}

private module FlowTestImpl implements InputSig<Location, RustDataFlow> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
identityLocalStep
| main.rs:404:7:404:18 | phi(default_name) | Node steps to itself |
| main.rs:394:7:394:18 | phi(default_name) | Node steps to itself |
Loading

0 comments on commit 87b9e60

Please sign in to comment.