Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
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
11 changes: 6 additions & 5 deletions python/ql/consistency-queries/DataFlowConsistency.ql
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ private import semmle.python.dataflow.new.internal.DataFlowImplSpecific
private import semmle.python.dataflow.new.internal.DataFlowDispatch
private import semmle.python.dataflow.new.internal.TaintTrackingImplSpecific
private import codeql.dataflow.internal.DataFlowImplConsistency
private import semmle.python.controlflow.internal.Cfg as Cfg

private module Input implements InputSig<Location, PythonDataFlow> {
private import Private
Expand Down Expand Up @@ -72,7 +73,7 @@ private module Input implements InputSig<Location, PythonDataFlow> {
// resolve to multiple functions), but we only make _one_ ArgumentNode for each
// argument in the CallNode, we end up violating this consistency check in those
// cases. (see `getCallArg` in DataFlowDispatch.qll)
exists(DataFlowCall other, CallNode cfgCall | other != call |
exists(DataFlowCall other, Cfg::CallNode cfgCall | other != call |
call.getNode() = cfgCall and
other.getNode() = cfgCall and
isArgumentNode(arg, call, _) and
Expand All @@ -88,16 +89,16 @@ private module Input implements InputSig<Location, PythonDataFlow> {
// allow it instead.
(
call.getScope() = attr.getScope() and
any(CfgNode n | n.asCfgNode() = call.getNode().(CallNode).getFunction()).getALocalSource() =
attr
any(CfgNode n | n.asCfgNode() = call.getNode().(Cfg::CallNode).getFunction())
.getALocalSource() = attr
or
not exists(call.getScope().(Function).getDefinition()) and
call.getScope().getScope+() = attr.getScope()
) and
(
other.getScope() = attr.getScope() and
any(CfgNode n | n.asCfgNode() = other.getNode().(CallNode).getFunction()).getALocalSource() =
attr
any(CfgNode n | n.asCfgNode() = other.getNode().(Cfg::CallNode).getFunction())
.getALocalSource() = attr
or
not exists(other.getScope().(Function).getDefinition()) and
other.getScope().getScope+() = attr.getScope()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: breaking
---
* The deprecated `AstNode.getAFlowNode()` and `Function.getAReturnValueFlowNode()` predicates now return nodes from the new shared CFG (`Cfg::ControlFlowNode`) rather than from the legacy CFG (`ControlFlowNode`). Callers that still rely on these deprecated APIs and feed the result into legacy-CFG-aware predicates will no longer type-check; migrate to `n.getNode() = e` (or, for return values, the explicit `Return` pattern shown in the deprecation message) to get nodes from the dataflow library's current CFG.
13 changes: 7 additions & 6 deletions python/ql/lib/semmle/python/ApiGraphs.qll
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
* directed and labeled; they specify how the components represented by nodes relate to each other.
*/

// Importing python under the `py` namespace to avoid importing `CallNode` from `Flow.qll` and thereby having a naming conflict with `API::CallNode`.
// Importing python under the `PY` namespace to avoid pulling in `CallNode` from `Flow.qll` (via `import python`) and thereby having a naming conflict with `API::CallNode`.
private import python as PY
private import semmle.python.controlflow.internal.Cfg as Cfg
import semmle.python.dataflow.new.DataFlow
private import semmle.python.internal.CachedStages

Expand Down Expand Up @@ -282,15 +283,15 @@ module API {
index = this.getIndex() and
(
// subscripting
exists(PY::SubscriptNode subscript |
exists(Cfg::SubscriptNode subscript |
subscript.getObject() = this.getAValueReachableFromSource().asCfgNode() and
subscript.getIndex() = index.asSink().asCfgNode()
|
// reading
subscript = result.asSource().asCfgNode()
or
// writing
subscript.(PY::DefinitionNode).getValue() = result.asSink().asCfgNode()
subscript.(Cfg::DefinitionNode).getValue() = result.asSink().asCfgNode()
)
or
// dictionary literals
Expand Down Expand Up @@ -684,7 +685,7 @@ module API {
* Ignores relative imports, such as `from ..foo.bar import baz`.
*/
private predicate imports(DataFlow::CfgNode imp, string name) {
exists(PY::ImportExprNode iexpr |
exists(Cfg::ImportExprNode iexpr |
imp.getNode() = iexpr and
not iexpr.getNode().isRelative() and
name = iexpr.getNode().getImportedModuleName()
Expand Down Expand Up @@ -775,7 +776,7 @@ module API {
// list literals, from `x` to `[x]`
// TODO: once convenient, this should be done at a higher level than the AST,
// at least at the CFG layer, to take splitting into account.
// Also consider `SequenceNode for generality.
// Also consider `Cfg::SequenceNode` for generality.
exists(PY::List list | list = pred.(DataFlow::ExprNode).getNode().getNode() |
Comment on lines 777 to 780
rhs.(DataFlow::ExprNode).getNode().getNode() = list.getAnElt() and
lbl = Label::subscript()
Expand Down Expand Up @@ -805,7 +806,7 @@ module API {
subscript = trackUseNode(src).getSubscript(index)
|
// from `x` to a definition of `x[...]`
rhs.asCfgNode() = subscript.asCfgNode().(PY::DefinitionNode).getValue() and
rhs.asCfgNode() = subscript.asCfgNode().(Cfg::DefinitionNode).getValue() and
lbl = Label::subscript()
or
// from `x` to `"key"` in `x["key"]`
Expand Down
14 changes: 7 additions & 7 deletions python/ql/lib/semmle/python/AstExtended.qll
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module;

import python
private import semmle.python.internal.CachedStages
private import semmle.python.controlflow.internal.Cfg as Cfg

/** A syntactic node (Class, Function, Module, Expr, Stmt or Comprehension) corresponding to a flow node */
abstract class AstNode extends AstNode_ {
Expand All @@ -23,17 +24,16 @@ abstract class AstNode extends AstNode_ {
/**
* DEPRECATED: use `ControlFlowNode.getNode()` from the other direction instead;
* that is, replace `e.getAFlowNode() = n` with `n.getNode() = e`. This API is
* being removed to untangle the AST and CFG hierarchies in preparation for
* migrating the dataflow library off the legacy CFG.
* being removed to untangle the AST and CFG hierarchies.
*
* Gets a flow node corresponding directly to this node.
* NOTE: For some statements and other purely syntactic elements,
* there may not be a `ControlFlowNode`.
* Gets a flow node corresponding directly to this node, from the new
* (shared) CFG. NOTE: For some statements and other purely syntactic
* elements, there may not be a `ControlFlowNode`.
*/
cached
deprecated ControlFlowNode getAFlowNode() {
deprecated Cfg::ControlFlowNode getAFlowNode() {
Stages::AST::ref() and
py_flow_bb_node(result, this, _, _)
result.getNode() = this
}

/**
Expand Down
5 changes: 3 additions & 2 deletions python/ql/lib/semmle/python/Concepts.qll
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

private import python
private import semmle.python.controlflow.internal.Cfg as Cfg
private import semmle.python.dataflow.new.DataFlow
private import semmle.python.dataflow.new.internal.DataFlowImplSpecific
private import semmle.python.dataflow.new.RemoteFlowSources
Expand Down Expand Up @@ -214,7 +215,7 @@ module Path {
SafeAccessCheck() { this = DataFlow::BarrierGuard<safeAccessCheck/3>::getABarrierNode() }
}

private predicate safeAccessCheck(DataFlow::GuardNode g, ControlFlowNode node, boolean branch) {
private predicate safeAccessCheck(DataFlow::GuardNode g, Cfg::ControlFlowNode node, boolean branch) {
g.(SafeAccessCheck::Range).checks(node, branch)
}

Expand All @@ -223,7 +224,7 @@ module Path {
/** A data-flow node that checks that a path is safe to access in some way, for example by having a controlled prefix. */
abstract class Range extends DataFlow::GuardNode {
/** Holds if this guard validates `node` upon evaluating to `branch`. */
abstract predicate checks(ControlFlowNode node, boolean branch);
abstract predicate checks(Cfg::ControlFlowNode node, boolean branch);
}
}
}
Expand Down
9 changes: 5 additions & 4 deletions python/ql/lib/semmle/python/Function.qll
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ overlay[local]
module;

import python
private import semmle.python.controlflow.internal.Cfg as Cfg

/**
* A function, independent of defaults and binding.
Expand Down Expand Up @@ -157,12 +158,12 @@ class Function extends Function_, Scope, AstNode {
* DEPRECATED: bind a `Return` node explicitly instead, e.g.
* `exists(Return ret | ret.getScope() = this and n.getNode() = ret.getValue())`.
* This API is being phased out together with `AstNode.getAFlowNode()` to
* untangle the AST and CFG hierarchies in preparation for migrating the
* dataflow library off the legacy CFG.
* untangle the AST and CFG hierarchies.
*
* Gets a control flow node for a return value of this function.
* Gets a control flow node for a return value of this function, from the
* new (shared) CFG.
*/
deprecated ControlFlowNode getAReturnValueFlowNode() {
deprecated Cfg::ControlFlowNode getAReturnValueFlowNode() {
exists(Return ret |
ret.getScope() = this and
ret.getValue() = result.getNode()
Expand Down
13 changes: 7 additions & 6 deletions python/ql/lib/semmle/python/dataflow/new/BarrierGuards.qll
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
/** Provides commonly used BarrierGuards. */

private import python
private import semmle.python.controlflow.internal.Cfg as Cfg
private import semmle.python.dataflow.new.DataFlow

private predicate constCompare(DataFlow::GuardNode g, ControlFlowNode node, boolean branch) {
exists(CompareNode cn | cn = g |
exists(ImmutableLiteral const, Cmpop op, ControlFlowNode c |
private predicate constCompare(DataFlow::GuardNode g, Cfg::ControlFlowNode node, boolean branch) {
exists(Cfg::CompareNode cn | cn = g |
exists(ImmutableLiteral const, Cmpop op, Cfg::ControlFlowNode c |
c.getNode() = const and
(
op = any(Eq eq) and branch = true
Expand All @@ -18,7 +19,7 @@ private predicate constCompare(DataFlow::GuardNode g, ControlFlowNode node, bool
cn.operands(node, op, c)
)
or
exists(NameConstant const, Cmpop op, ControlFlowNode c |
exists(NameConstant const, Cmpop op, Cfg::ControlFlowNode c |
c.getNode() = const and
(
op = any(Is is_) and branch = true
Expand All @@ -31,12 +32,12 @@ private predicate constCompare(DataFlow::GuardNode g, ControlFlowNode node, bool
cn.operands(node, op, c)
)
or
exists(IterableNode const_iterable, Cmpop op |
exists(Cfg::IterableNode const_iterable, Cmpop op |
op = any(In in_) and branch = true
or
op = any(NotIn ni) and branch = false
|
forall(ControlFlowNode elem | elem = const_iterable.getAnElement() |
forall(Cfg::ControlFlowNode elem | elem = const_iterable.getAnElement() |
elem.getNode() instanceof ImmutableLiteral
) and
cn.operands(node, op, const_iterable)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

private import python
private import semmle.python.controlflow.internal.Cfg as Cfg
private import semmle.python.dataflow.new.DataFlow
// Need to import `semmle.python.Frameworks` since frameworks can extend `SensitiveDataSource::Range`
private import semmle.python.Frameworks
Expand Down Expand Up @@ -105,7 +106,7 @@ private module SensitiveDataModeling {
or
// to cover functions that we don't have the definition for, and where the
// reference to the function has not already been marked as being sensitive
this.getFunction().asCfgNode().(NameNode).getId() = sensitiveString(classification)
this.getFunction().asCfgNode().(Cfg::NameNode).getId() = sensitiveString(classification)
}

override SensitiveDataClassification getClassification() { result = classification }
Expand Down Expand Up @@ -251,12 +252,12 @@ private module SensitiveDataModeling {
SensitiveDataClassification classification;

SensitiveVariableAssignment() {
exists(DefinitionNode def |
def.(NameNode).getId() = sensitiveString(classification) and
exists(Cfg::DefinitionNode def |
def.(Cfg::NameNode).getId() = sensitiveString(classification) and
(
this.asCfgNode() = def.getValue()
or
this.asCfgNode() = def.getValue().(ForNode).getSequence()
this.asCfgNode() = def.getValue().(Cfg::ForNode).getSequence()
) and
not this.asExpr() instanceof FunctionExpr and
not this.asExpr() instanceof ClassExpr
Expand Down Expand Up @@ -293,7 +294,7 @@ private module SensitiveDataModeling {
SensitiveDataClassification classification;

SensitiveSubscript() {
this.asCfgNode().(SubscriptNode).getIndex() =
this.asCfgNode().(Cfg::SubscriptNode).getIndex() =
sensitiveLookupStringConst(classification).asCfgNode()
}

Expand Down
29 changes: 15 additions & 14 deletions python/ql/lib/semmle/python/dataflow/new/internal/Attributes.qll
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ overlay[local]
module;

private import python
private import semmle.python.controlflow.internal.Cfg as Cfg
import DataFlowUtil
import DataFlowPublic
private import DataFlowPrivate
Expand Down Expand Up @@ -83,9 +84,9 @@ abstract class AttrWrite extends AttrRef {
* ```python
* object.attr = value
* ```
* Also gives access to the `value` being written, by extending `DefinitionNode`.
* Also gives access to the `value` being written, by extending `Cfg::DefinitionNode`.
*/
private class AttributeAssignmentNode extends DefinitionNode, AttrNode { }
private class AttributeAssignmentNode extends Cfg::DefinitionNode, Cfg::AttrNode { }

/** A simple attribute assignment: `object.attr = value`. */
private class AttributeAssignmentAsAttrWrite extends AttrWrite, CfgNode {
Expand Down Expand Up @@ -131,21 +132,21 @@ private class GlobalAttributeAssignmentAsAttrWrite extends AttrWrite, CfgNode {
override string getAttributeName() { result = node.getName() }
}

/** Represents `CallNode`s that may refer to calls to built-in functions or classes. */
private class BuiltInCallNode extends CallNode {
/** Represents `Cfg::CallNode`s that may refer to calls to built-in functions or classes. */
private class BuiltInCallNode extends Cfg::CallNode {
string name;

BuiltInCallNode() {
// TODO disallow instances where the name of the built-in may refer to an in-scope variable of that name.
exists(NameNode id |
exists(Cfg::NameNode id |
name = Builtins::getBuiltinName() and
this.getFunction() = id and
id.getId() = name and
id.isGlobal()
)
}

/** Gets the name of the built-in function that is called at this `CallNode` */
/** Gets the name of the built-in function that is called at this `Cfg::CallNode` */
string getBuiltinName() { result = name }
}

Expand All @@ -157,20 +158,20 @@ private class BuiltinAttrCallNode extends BuiltInCallNode {
BuiltinAttrCallNode() { name in ["setattr", "getattr", "hasattr", "delattr"] }

/** Gets the control flow node for object on which the attribute is accessed. */
ControlFlowNode getObject() { result in [this.getArg(0), this.getArgByName("object")] }
Cfg::ControlFlowNode getObject() { result in [this.getArg(0), this.getArgByName("object")] }

/**
* Gets the control flow node for the value that is being written to the attribute.
* Only relevant for `setattr` calls.
*/
ControlFlowNode getValue() {
Cfg::ControlFlowNode getValue() {
// only valid for `setattr`
name = "setattr" and
result in [this.getArg(2), this.getArgByName("value")]
}

/** Gets the control flow node that defines the name of the attribute being accessed. */
ControlFlowNode getName() { result in [this.getArg(1), this.getArgByName("name")] }
Cfg::ControlFlowNode getName() { result in [this.getArg(1), this.getArgByName("name")] }
}

/** Represents calls to the built-in `setattr`. */
Expand Down Expand Up @@ -205,10 +206,10 @@ private class SetAttrCallAsAttrWrite extends AttrWrite, CfgNode {
* attr = value
* ...
* ```
* Instances of this class correspond to the `NameNode` for `attr`, and also gives access to `value` by
* virtue of being a `DefinitionNode`.
* Instances of this class correspond to the `Cfg::NameNode` for `attr`, and also gives access to `value` by
* virtue of being a `Cfg::DefinitionNode`.
*/
private class ClassAttributeAssignmentNode extends DefinitionNode, NameNode {
private class ClassAttributeAssignmentNode extends Cfg::DefinitionNode, Cfg::NameNode {
ClassAttributeAssignmentNode() { this.getScope() = any(ClassExpr c).getInnerScope() }
}

Expand Down Expand Up @@ -248,7 +249,7 @@ abstract class AttrRead extends AttrRef, Node, LocalSourceNode {

/** A simple attribute read, e.g. `object.attr` */
private class AttributeReadAsAttrRead extends AttrRead, CfgNode {
override AttrNode node;
override Cfg::AttrNode node;

AttributeReadAsAttrRead() { node.isLoad() }

Expand Down Expand Up @@ -285,7 +286,7 @@ private class GetAttrCallAsAttrRead extends AttrRead, CfgNode {
* is treated as if it is a read of the attribute `module.attr`, even if `module` is not imported directly.
*/
private class ModuleAttributeImportAsAttrRead extends AttrRead, CfgNode {
override ImportMemberNode node;
override Cfg::ImportMemberNode node;

override Node getObject() { result.asCfgNode() = node.getModule(_) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ overlay[local]
module;

private import python
private import semmle.python.controlflow.internal.Cfg as Cfg
private import semmle.python.dataflow.new.DataFlow
private import semmle.python.dataflow.new.internal.ImportStar

Expand Down Expand Up @@ -67,7 +68,7 @@ module Builtins {
DataFlow::CfgNode likelyBuiltin(string name) {
exists(Module m |
result.getNode() =
any(NameNode n |
any(Cfg::NameNode n |
possible_builtin_accessed_in_module(n, name, m) and
not possible_builtin_defined_in_module(name, m)
)
Expand All @@ -87,7 +88,7 @@ module Builtins {
* Holds if `n` is an access of a global variable called `name` (which is also the name of a
* built-in) inside the module `m`.
*/
private predicate possible_builtin_accessed_in_module(NameNode n, string name, Module m) {
private predicate possible_builtin_accessed_in_module(Cfg::NameNode n, string name, Module m) {
n.isGlobal() and
n.isLoad() and
name = n.getId() and
Expand Down
Loading
Loading