Merge pull request #9126 from RasmusWL/moduleimport-with-dots

Python: Fully disallow `API::moduleImport` of module with dots

python/ql/lib/change-notes/2022-05-12-moduleimport-disallow-dots.md

@@ -0,0 +1,4 @@
+---
+category: breaking
+---
+`API::moduleImport` no longer has any results for dotted names, such as `API::moduleImport("foo.bar")`. Using `API::moduleImport("foo.bar").getMember("baz").getACall()` previously worked if the Python code was `from foo.bar import baz; baz()`, but not if the code was `import foo.bar; foo.bar.baz()` -- we are making this change to ensure the approach that can handle all cases is always used.

python/ql/lib/semmle/python/ApiGraphs.qll

@@ -280,7 +280,13 @@ module API {
    * you should use `.getMember` on the parent module. For example, for nodes corresponding to the module `foo.bar`,
    * use `moduleImport("foo").getMember("bar")`.
    */
-  Node moduleImport(string m) { result = Impl::MkModuleImport(m) }
+  Node moduleImport(string m) {
+    result = Impl::MkModuleImport(m) and
+    // restrict `moduleImport` so it will never give results for a dotted name. Note
+    // that we cannot move this logic to the `MkModuleImport` construction, since we
+    // need the intermediate API graph nodes for the prefixes in `import foo.bar.baz`.
+    not m.matches("%.%")
+  }
 
   /** Gets a node corresponding to the built-in with the given name, if any. */
   Node builtin(string n) { result = moduleImport("builtins").getMember(n) }

python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql

@@ -16,7 +16,7 @@ import semmle.python.dataflow.new.TaintTracking
 
 API::Node libPam() {
   exists(API::CallNode findLibCall, API::CallNode cdllCall |
-    findLibCall = API::moduleImport("ctypes.util").getMember("find_library").getACall() and
+    findLibCall = API::moduleImport("ctypes").getMember("util").getMember("find_library").getACall() and
     findLibCall.getParameter(0).getAValueReachingRhs().asExpr().(StrConst).getText() = "pam" and
     cdllCall = API::moduleImport("ctypes").getMember("CDLL").getACall() and
     cdllCall.getParameter(0).getAValueReachingRhs() = findLibCall

python/ql/src/experimental/semmle/python/frameworks/NoSQL.qll

@@ -210,10 +210,13 @@ private module NoSql {
    */
   private class BsonObjectIdCall extends DataFlow::CallCfgNode, NoSqlSanitizer::Range {
     BsonObjectIdCall() {
-      this =
-        API::moduleImport(["bson", "bson.objectid", "bson.json_util"])
-            .getMember("ObjectId")
-            .getACall()
+      exists(API::Node mod |
+        mod = API::moduleImport("bson")
+        or
+        mod = API::moduleImport("bson").getMember(["objectid", "json_util"])
+      |
+        this = mod.getMember("ObjectId").getACall()
+      )
     }
 
     override DataFlow::Node getAnInput() { result = this.getArg(0) }

python/ql/test/experimental/dataflow/typetracking/tracked.ql

@@ -131,7 +131,7 @@ DataFlow::Node foo() { foo(DataFlow::TypeTracker::end()).flowsTo(result) }
 /** Gets a reference to `foo.bar` (fictive module). */
 private DataFlow::TypeTrackingNode foo_bar(DataFlow::TypeTracker t) {
   t.start() and
-  result = API::moduleImport("foo.bar").getAnImmediateUse()
+  result = API::moduleImport("foo").getMember("bar").getAnImmediateUse()
   or
   t.startInAttr("bar") and
   result = foo()
@@ -145,7 +145,7 @@ DataFlow::Node foo_bar() { foo_bar(DataFlow::TypeTracker::end()).flowsTo(result)
 /** Gets a reference to `foo.bar.baz` (fictive attribute on `foo.bar` module). */
 private DataFlow::TypeTrackingNode foo_bar_baz(DataFlow::TypeTracker t) {
   t.start() and
-  result = API::moduleImport("foo.bar.baz").getAnImmediateUse()
+  result = API::moduleImport("foo").getMember("bar").getMember("baz").getAnImmediateUse()
   or
   t.startInAttr("baz") and
   result = foo_bar()

python/ql/test/library-tests/ApiGraphs/py3/ModuleImportWithDots.expected

@@ -0,0 +1,5 @@
+moduleImportWithDots
+doesntFullyWork
+works
+| test.py:25:6:25:18 | ControlFlowNode for Attribute() |
+| test.py:28:10:28:17 | ControlFlowNode for method() |

python/ql/test/library-tests/ApiGraphs/py3/ModuleImportWithDots.ql

@@ -0,0 +1,18 @@
+private import python
+private import semmle.python.ApiGraphs
+
+query API::Node moduleImportWithDots() { result = API::moduleImport("a.b.c.d") }
+
+query API::CallNode doesntFullyWork() {
+  result = API::moduleImport("a.b.c.d").getMember("method").getACall()
+}
+
+query API::CallNode works() {
+  result =
+    API::moduleImport("a")
+        .getMember("b")
+        .getMember("c")
+        .getMember("d")
+        .getMember("method")
+        .getACall()
+}

python/ql/test/library-tests/ApiGraphs/py3/test.py

@@ -24,6 +24,9 @@ abcd = abc.d #$ use=moduleImport("a").getMember("b").getMember("c").getMember("d
 
 x5 = abcd.method() #$ use=moduleImport("a").getMember("b").getMember("c").getMember("d").getMember("method").getReturn()
 
+from a.b.c.d import method
+x5_alt = method() #$ use=moduleImport("a").getMember("b").getMember("c").getMember("d").getMember("method").getReturn()
+
 from a6 import m6 #$ use=moduleImport("a6").getMember("m6")
 
 x6 = m6().foo().bar() #$ use=moduleImport("a6").getMember("m6").getReturn().getMember("foo").getReturn().getMember("bar").getReturn()

python/ql/test/library-tests/ApiGraphs/py3/verifyApiGraphs.ql

@@ -1 +1,3 @@
+// Note: This is not using standard inline-expectation tests, so will not alert if you
+// have not manually added an annotation to a line!
 import TestUtilities.VerifyApiGraphs