Merge pull request #1487 from markshannon/python-tuple-assignment-points-to

Python ESSA dataflow: better handling of tuple unpacking.

Author: taus-semmle

python/ql/src/semmle/python/Flow.qll

@@ -745,6 +745,8 @@ class DefinitionNode extends ControlFlowNode {
         exists(Assign a | a.getATarget().(Tuple).getAnElt().getAFlowNode() = this)
         or
         exists(Assign a | a.getATarget().(List).getAnElt().getAFlowNode() = this)
+        or
+        exists(For for | for.getTarget().getAFlowNode() = this)
     }
 
     /** flow node corresponding to the value assigned for the definition corresponding to this flow node */
@@ -864,7 +866,7 @@ class DictNode extends ControlFlowNode {
 
 }
 
-private Expr assigned_value(Expr lhs) {
+private AstNode assigned_value(Expr lhs) {
     /* lhs = result */
     exists(Assign a | a.getATarget() = lhs and result = a.getValue())
     or
@@ -881,6 +883,9 @@ private Expr assigned_value(Expr lhs) {
         lhs = target.getElt(index) and
         result = values.getElt(index)
     )
+    or
+    /* for lhs in seq: => `result` is the `for` node, representing the `iter(next(seq))` operation. */
+    result.(For).getTarget() = lhs
 }
 
 /** A flow node for a `for` statement. */

python/ql/src/semmle/python/dataflow/SsaDefinitions.qll

@@ -58,12 +58,10 @@ abstract class PythonSsaSourceVariable extends SsaSourceVariable {
         or
         SsaSource::assignment_definition(this, def, _)
         or
-        SsaSource::multi_assignment_definition(this, def)
+        SsaSource::multi_assignment_definition(this, def, _, _)
         or
         SsaSource::deletion_definition(this, def)
         or
-        SsaSource::iteration_defined_variable(this, def, _)
-        or
         SsaSource::init_module_submodule_defn(this, def)
         or
         SsaSource::parameter_definition(this, def)
@@ -381,10 +379,11 @@ cached module SsaSource {
     }
 
     /** Holds if `v` is defined by multiple assignment at `defn`. */
-    cached predicate multi_assignment_definition(Variable v, ControlFlowNode defn) {
+    cached predicate multi_assignment_definition(Variable v, ControlFlowNode defn, int n, SequenceNode lhs) {
         defn.(NameNode).defines(v) and 
         not exists(defn.(DefinitionNode).getValue()) and
-        exists(SequenceNode s | s.getAnElement() = defn)
+        lhs.getElement(n) = defn and
+        lhs.getBasicBlock().dominates(defn.getBasicBlock())
     }
 
     /** Holds if `v` is defined by a `for` statement, the definition being `defn` */

python/ql/src/semmle/python/objects/Callables.qll

@@ -48,6 +48,9 @@ abstract class CallableObjectInternal extends ObjectInternal {
 
     override string strValue() { none() }
 
+    /* Callables aren't iterable */
+    override ObjectInternal getIterNext() { none() }
+
 }
 
 /** Class representing Python functions */

python/ql/src/semmle/python/objects/Classes.qll

@@ -89,6 +89,10 @@ abstract class ClassObjectInternal extends ObjectInternal {
     }
 
     override predicate subscriptUnknown() { none() }
+
+    /* Classes aren't usually iterable, but can e.g. Enums */
+    override ObjectInternal getIterNext() { result = ObjectInternal::unknown() }
+
 }
 
 /** Class representing Python source classes */

python/ql/src/semmle/python/objects/Constants.qll

@@ -83,6 +83,9 @@ private abstract class BooleanObjectInternal extends ConstantObjectInternal {
         none()
     }
 
+    /* Booleans aren't iterable */
+    override ObjectInternal getIterNext() { none() }
+
 }
 
 private class TrueObjectInternal extends BooleanObjectInternal, TTrue {
@@ -165,6 +168,9 @@ private class NoneObjectInternal extends ConstantObjectInternal, TNone {
 
     override int length() { none() }
 
+    /* None isn't iterable */
+    override ObjectInternal getIterNext() { none() }
+
 }
 
 
@@ -203,6 +209,9 @@ private class IntObjectInternal extends ConstantObjectInternal, TInt {
 
     override int length() { none() }
 
+    /* ints aren't iterable */
+    override ObjectInternal getIterNext() { none() }
+
 }
 
 private class FloatObjectInternal extends ConstantObjectInternal, TFloat {
@@ -248,6 +257,9 @@ private class FloatObjectInternal extends ConstantObjectInternal, TFloat {
 
     override int length() { none() }
 
+    /* floats aren't iterable */
+    override ObjectInternal getIterNext() { none() }
+
 }
 
 
@@ -290,6 +302,10 @@ private class UnicodeObjectInternal extends ConstantObjectInternal, TUnicode {
         result = this.strValue().length()
     }
 
+    override ObjectInternal getIterNext() {
+        result = TUnknownInstance(this.getClass())
+    }
+
 }
 
 private class BytesObjectInternal extends ConstantObjectInternal, TBytes {
@@ -331,6 +347,10 @@ private class BytesObjectInternal extends ConstantObjectInternal, TBytes {
         result = this.strValue().length()
     }
 
+    override ObjectInternal getIterNext() {
+        result = TUnknownInstance(this.getClass())
+    }
+
 }
 
 

python/ql/src/semmle/python/objects/Descriptors.qll

@@ -91,6 +91,9 @@ class PropertyInternal extends ObjectInternal, TProperty {
         )
     }
 
+    /* Properties aren't iterable */
+    override ObjectInternal getIterNext() { none() }
+
 }
 
 /** A class representing classmethods in Python */
@@ -176,6 +179,9 @@ class ClassMethodObjectInternal extends ObjectInternal, TClassMethod {
         result = this.getFunction().getName()
     }
 
+    /* Classmethods aren't iterable */
+    override ObjectInternal getIterNext() { none() }
+
 }
 
 class StaticMethodObjectInternal extends ObjectInternal, TStaticMethod {
@@ -247,4 +253,7 @@ class StaticMethodObjectInternal extends ObjectInternal, TStaticMethod {
         result = this.getFunction().getName()
     }
 
+    /* Staticmethods aren't iterable */
+    override ObjectInternal getIterNext() { none() }
+
 }

python/ql/src/semmle/python/objects/Instances.qll

@@ -51,6 +51,8 @@ abstract class InstanceObject extends ObjectInternal {
 
     override string getName() { none() }
 
+    override ObjectInternal getIterNext() { result = ObjectInternal::unknown() }
+
 }
 
 private predicate self_variable_reaching_init_exit(EssaVariable self) {
@@ -366,6 +368,8 @@ class UnknownInstanceInternal extends TUnknownInstance, ObjectInternal {
 
     override string getName() { none() }
 
+    override ObjectInternal getIterNext() { result = ObjectInternal::unknown() }
+
 }
 
 private int lengthFromClass(ClassObjectInternal cls) {
@@ -472,5 +476,7 @@ class SuperInstance extends TSuperInstance, ObjectInternal {
 
     override string getName() { none() }
 
+    override ObjectInternal getIterNext() { result = ObjectInternal::unknown() }
+
 }
 

python/ql/src/semmle/python/objects/Modules.qll

@@ -52,6 +52,9 @@ abstract class ModuleObjectInternal extends ObjectInternal {
         any(PackageObjectInternal package).getInitModule() = this
     }
 
+    /* Modules aren't iterable */
+    override ObjectInternal getIterNext() { none() }
+
 }
 
 /** A class representing built-in modules */
@@ -408,5 +411,8 @@ class AbsentModuleAttributeObjectInternal extends ObjectInternal, TAbsentModuleA
     /* We know what this is called, but not its innate name */
     override string getName() { none() }
 
+    /* Modules aren't iterable */
+    override ObjectInternal getIterNext() { none() }
+
 }
 

python/ql/src/semmle/python/objects/ObjectInternal.qll

@@ -167,6 +167,12 @@ class ObjectInternal extends TObject {
      */
     abstract string getName();
 
+    /** Gets the 'object' resulting from iterating over this object.
+     * Used in the context `for i in this:`. The result is the 'object'
+     * assigned to `i`.
+     */
+    abstract ObjectInternal getIterNext();
+
 }
 
 
@@ -249,6 +255,9 @@ class BuiltinOpaqueObjectInternal extends ObjectInternal, TBuiltinOpaqueObject {
     override string getName() {
         result = this.getBuiltin().getName()
     }
+
+    override ObjectInternal getIterNext() { result = ObjectInternal::unknown() }
+
 }
 
 
@@ -326,6 +335,8 @@ class UnknownInternal extends ObjectInternal, TUnknown {
 
     override string getName() { none() }
 
+    override ObjectInternal getIterNext() { result = ObjectInternal::unknown() }
+
 }
 
 class UndefinedInternal extends ObjectInternal, TUndefined {
@@ -404,6 +415,8 @@ class UndefinedInternal extends ObjectInternal, TUndefined {
 
     override string getName() { none() }
 
+    override ObjectInternal getIterNext() { none() }
+
 }
 
 module ObjectInternal {

python/ql/src/semmle/python/objects/Sequences.qll

@@ -32,6 +32,8 @@ abstract class SequenceObjectInternal extends ObjectInternal {
 
     override string getName() { none() }
 
+    override ObjectInternal getIterNext() { result = this.getItem(_) }
+
 }
 
 abstract class TupleObjectInternal extends SequenceObjectInternal {