Fix MISSING alert

python/ql/lib/semmle/python/dataflow/new/internal/TypeTrackingImpl.qll

@@ -167,7 +167,9 @@ module TypeTrackingInput implements Shared::TypeTrackingInput<Location> {
   }
 
   /** Holds if there is a level step from `nodeFrom` to `nodeTo`, which may depend on the call graph. */
-  predicate levelStepCall(Node nodeFrom, LocalSourceNode nodeTo) { none() }
+  predicate levelStepCall(Node nodeFrom, LocalSourceNode nodeTo) {
+    instanceFieldStep(nodeFrom, nodeTo)
+  }
 
   /** Holds if there is a level step from `nodeFrom` to `nodeTo`, which does not depend on the call graph. */
   predicate levelStepNoCall(Node nodeFrom, LocalSourceNode nodeTo) {
@@ -337,6 +339,20 @@ module TypeTrackingInput implements Shared::TypeTrackingInput<Location> {
     )
   }
 
+  /**
+   * Holds if `read` reads attribute `attr` from an instance of `cls`, where the instance
+   * is referred to from outside the methods of `cls` (i.e. an access of the form
+   * `instance.attr`, where `instance` is a reference to an instance of `cls`).
+   *
+   * This complements `selfAttrRef`, which only handles `self.attr` accesses inside the
+   * methods of `cls`. Unlike `selfAttrRef`, this depends on the call graph (via
+   * `classInstanceTracker`), so steps using it must be reported as `levelStepCall`.
+   */
+  private predicate instanceAttrRead(Class cls, string attr, DataFlowPublic::AttrRead read) {
+    read.getObject() = DataFlowDispatch::classInstanceTracker(cls) and
+    read.mayHaveAttributeName(attr)
+  }
+
   /**
    * Holds if `nodeFrom` is written to attribute `self.attr` in some instance method of a
    * class, and `nodeTo` reads attribute `self.attr` in some (possibly different) instance
@@ -364,6 +380,28 @@ module TypeTrackingInput implements Shared::TypeTrackingInput<Location> {
     )
   }
 
+  /**
+   * Holds if `nodeFrom` is written to attribute `self.attr` in some instance method of a
+   * class, and `nodeTo` reads attribute `attr` from an instance of the same class outside
+   * its methods (e.g. `instance.attr`).
+   *
+   * This is the cross-instance counterpart of `localFieldStep`: it relates a write of
+   * `self.attr` inside the class to a read of `attr` on a reference to an instance of the
+   * class. Identifying instances relies on the call graph (via `classInstanceTracker`), so
+   * this step is reported as `levelStepCall` rather than `levelStepNoCall`.
+   *
+   * Like `localFieldStep`, this is an over-approximation: it is both instance-insensitive
+   * and order-insensitive.
+   */
+  private predicate instanceFieldStep(Node nodeFrom, LocalSourceNode nodeTo) {
+    exists(Class cls, string attr, DataFlowPublic::AttrWrite write, DataFlowPublic::AttrRead read |
+      selfAttrRef(cls, attr, write) and
+      nodeFrom = write.getValue() and
+      instanceAttrRead(cls, attr, read) and
+      nodeTo = read
+    )
+  }
+
   /**
    * Holds if data can flow from `node1` to `node2` in a way that discards call contexts.
    */

python/ql/test/query-tests/Security/CWE-089-SqlInjection/SqlInjection.expected

@@ -1,4 +1,9 @@
 #select
+| app.py:23:20:23:24 | ControlFlowNode for query | app.py:20:18:20:21 | ControlFlowNode for name | app.py:23:20:23:24 | ControlFlowNode for query | This SQL query depends on a $@. | app.py:20:18:20:21 | ControlFlowNode for name | user-provided value |
+| app.py:30:20:30:24 | ControlFlowNode for query | app.py:27:19:27:22 | ControlFlowNode for name | app.py:30:20:30:24 | ControlFlowNode for query | This SQL query depends on a $@. | app.py:27:19:27:22 | ControlFlowNode for name | user-provided value |
+| app.py:37:20:37:24 | ControlFlowNode for query | app.py:34:19:34:22 | ControlFlowNode for name | app.py:37:20:37:24 | ControlFlowNode for query | This SQL query depends on a $@. | app.py:34:19:34:22 | ControlFlowNode for name | user-provided value |
+| app.py:44:20:44:24 | ControlFlowNode for query | app.py:41:19:41:22 | ControlFlowNode for name | app.py:44:20:44:24 | ControlFlowNode for query | This SQL query depends on a $@. | app.py:41:19:41:22 | ControlFlowNode for name | user-provided value |
+| app.py:51:20:51:24 | ControlFlowNode for query | app.py:48:19:48:22 | ControlFlowNode for name | app.py:51:20:51:24 | ControlFlowNode for query | This SQL query depends on a $@. | app.py:48:19:48:22 | ControlFlowNode for name | user-provided value |
 | sql_injection.py:21:24:21:77 | ControlFlowNode for BinaryExpr | sql_injection.py:14:15:14:22 | ControlFlowNode for username | sql_injection.py:21:24:21:77 | ControlFlowNode for BinaryExpr | This SQL query depends on a $@. | sql_injection.py:14:15:14:22 | ControlFlowNode for username | user-provided value |
 | sql_injection.py:24:38:24:95 | ControlFlowNode for BinaryExpr | sql_injection.py:14:15:14:22 | ControlFlowNode for username | sql_injection.py:24:38:24:95 | ControlFlowNode for BinaryExpr | This SQL query depends on a $@. | sql_injection.py:14:15:14:22 | ControlFlowNode for username | user-provided value |
 | sql_injection.py:25:26:25:83 | ControlFlowNode for BinaryExpr | sql_injection.py:14:15:14:22 | ControlFlowNode for username | sql_injection.py:25:26:25:83 | ControlFlowNode for BinaryExpr | This SQL query depends on a $@. | sql_injection.py:14:15:14:22 | ControlFlowNode for username | user-provided value |
@@ -16,6 +21,16 @@
 | sqlalchemy_textclause.py:50:18:50:25 | ControlFlowNode for username | sqlalchemy_textclause.py:23:15:23:22 | ControlFlowNode for username | sqlalchemy_textclause.py:50:18:50:25 | ControlFlowNode for username | This SQL query depends on a $@. | sqlalchemy_textclause.py:23:15:23:22 | ControlFlowNode for username | user-provided value |
 | sqlalchemy_textclause.py:51:24:51:31 | ControlFlowNode for username | sqlalchemy_textclause.py:23:15:23:22 | ControlFlowNode for username | sqlalchemy_textclause.py:51:24:51:31 | ControlFlowNode for username | This SQL query depends on a $@. | sqlalchemy_textclause.py:23:15:23:22 | ControlFlowNode for username | user-provided value |
 edges
+| app.py:20:18:20:21 | ControlFlowNode for name | app.py:21:5:21:9 | ControlFlowNode for query | provenance |  |
+| app.py:21:5:21:9 | ControlFlowNode for query | app.py:23:20:23:24 | ControlFlowNode for query | provenance |  |
+| app.py:27:19:27:22 | ControlFlowNode for name | app.py:28:5:28:9 | ControlFlowNode for query | provenance |  |
+| app.py:28:5:28:9 | ControlFlowNode for query | app.py:30:20:30:24 | ControlFlowNode for query | provenance |  |
+| app.py:34:19:34:22 | ControlFlowNode for name | app.py:35:5:35:9 | ControlFlowNode for query | provenance |  |
+| app.py:35:5:35:9 | ControlFlowNode for query | app.py:37:20:37:24 | ControlFlowNode for query | provenance |  |
+| app.py:41:19:41:22 | ControlFlowNode for name | app.py:42:5:42:9 | ControlFlowNode for query | provenance |  |
+| app.py:42:5:42:9 | ControlFlowNode for query | app.py:44:20:44:24 | ControlFlowNode for query | provenance |  |
+| app.py:48:19:48:22 | ControlFlowNode for name | app.py:49:5:49:9 | ControlFlowNode for query | provenance |  |
+| app.py:49:5:49:9 | ControlFlowNode for query | app.py:51:20:51:24 | ControlFlowNode for query | provenance |  |
 | sql_injection.py:14:15:14:22 | ControlFlowNode for username | sql_injection.py:21:24:21:77 | ControlFlowNode for BinaryExpr | provenance |  |
 | sql_injection.py:14:15:14:22 | ControlFlowNode for username | sql_injection.py:24:38:24:95 | ControlFlowNode for BinaryExpr | provenance |  |
 | sql_injection.py:14:15:14:22 | ControlFlowNode for username | sql_injection.py:25:26:25:83 | ControlFlowNode for BinaryExpr | provenance |  |
@@ -33,6 +48,21 @@ edges
 | sqlalchemy_textclause.py:23:15:23:22 | ControlFlowNode for username | sqlalchemy_textclause.py:50:18:50:25 | ControlFlowNode for username | provenance |  |
 | sqlalchemy_textclause.py:23:15:23:22 | ControlFlowNode for username | sqlalchemy_textclause.py:51:24:51:31 | ControlFlowNode for username | provenance |  |
 nodes
+| app.py:20:18:20:21 | ControlFlowNode for name | semmle.label | ControlFlowNode for name |
+| app.py:21:5:21:9 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
+| app.py:23:20:23:24 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
+| app.py:27:19:27:22 | ControlFlowNode for name | semmle.label | ControlFlowNode for name |
+| app.py:28:5:28:9 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
+| app.py:30:20:30:24 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
+| app.py:34:19:34:22 | ControlFlowNode for name | semmle.label | ControlFlowNode for name |
+| app.py:35:5:35:9 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
+| app.py:37:20:37:24 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
+| app.py:41:19:41:22 | ControlFlowNode for name | semmle.label | ControlFlowNode for name |
+| app.py:42:5:42:9 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
+| app.py:44:20:44:24 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
+| app.py:48:19:48:22 | ControlFlowNode for name | semmle.label | ControlFlowNode for name |
+| app.py:49:5:49:9 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
+| app.py:51:20:51:24 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
 | sql_injection.py:14:15:14:22 | ControlFlowNode for username | semmle.label | ControlFlowNode for username |
 | sql_injection.py:21:24:21:77 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
 | sql_injection.py:24:38:24:95 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |

python/ql/test/query-tests/Security/CWE-089-SqlInjection/app.py

@@ -31,10 +31,10 @@ async def unsafe2(name: str): # $ Source
     cursor.close()
 
 @app.get("/unsafe3/")
-async def unsafe3(name: str): # $ MISSING: Source
+async def unsafe3(name: str): # $ Source
     query = "select * from users where name=" + name
     cursor = hdb_con3.cursor()
-    cursor.execute(query) # $ MISSING: Alert
+    cursor.execute(query) # $ Alert
     cursor.close()
 
 @app.get("/unsafe4/")