Python: Model os.path.join and add taint-step

RasmusWL · RasmusWL · commit 0542c3b91e29 · 2020-09-30T11:42:36.000+02:00
diff --git a/python/ql/src/experimental/dataflow/internal/TaintTrackingPrivate.qll b/python/ql/src/experimental/dataflow/internal/TaintTrackingPrivate.qll
@@ -130,7 +130,6 @@ predicate stringManipulation(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeT
   // f-strings
   nodeTo.asExpr().(Fstring).getAValue() = nodeFrom.asExpr()
   // TODO: Handle encode/decode from base64/quopri
-  // TODO: Handle os.path.join
   // TODO: Handle functions in https://docs.python.org/3/library/binascii.html
 }
 
diff --git a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll
@@ -5,9 +5,11 @@
 
 private import python
 private import experimental.dataflow.DataFlow
+private import experimental.dataflow.TaintTracking
 private import experimental.dataflow.RemoteFlowSources
 private import experimental.semmle.python.Concepts
 
+/** Provides models for the Python standard library. */
 private module Stdlib {
   /** Gets a reference to the `os` module. */
   DataFlow::Node os(DataFlow::TypeTracker t) {
@@ -20,6 +22,7 @@ private module Stdlib {
   /** Gets a reference to the `os` module. */
   DataFlow::Node os() { result = os(DataFlow::TypeTracker::end()) }
 
+  /** Provides models for the `os` module. */
   module os {
     /** Gets a reference to the `os.system` function. */
     DataFlow::Node system(DataFlow::TypeTracker t) {
@@ -48,6 +51,41 @@ private module Stdlib {
 
     /** Gets a reference to the `os.popen` function. */
     DataFlow::Node popen() { result = os::popen(DataFlow::TypeTracker::end()) }
+
+    /** Gets a reference to the `os.path` module. */
+    private DataFlow::Node path(DataFlow::TypeTracker t) {
+      t.start() and
+      (
+        result = DataFlow::importMember("os", "path")
+        or
+        result = DataFlow::importModule("os.path")
+      )
+      or
+      t.startInAttr("path") and
+      result = os()
+      or
+      exists(DataFlow::TypeTracker t2 | result = path(t2).track(t2, t))
+    }
+
+    /** Gets a reference to the `os.path` module. */
+    DataFlow::Node path() { result = path(DataFlow::TypeTracker::end()) }
+
+    /** Provides models for the `os.path` module */
+    module path {
+      /** Gets a reference to the `os.path.join` function. */
+      private DataFlow::Node join(DataFlow::TypeTracker t) {
+        t.start() and
+        result = DataFlow::importMember("os.path", "join")
+        or
+        t.startInAttr("join") and
+        result = os::path()
+        or
+        exists(DataFlow::TypeTracker t2 | result = join(t2).track(t2, t))
+      }
+
+      /** Gets a reference to the `os.join` module. */
+      DataFlow::Node join() { result = join(DataFlow::TypeTracker::end()) }
+    }
   }
 
   /**
@@ -73,4 +111,16 @@ private module Stdlib {
       result.asCfgNode() = this.asCfgNode().(CallNode).getArg(0)
     }
   }
+
+  /** An additional taint step for calls to `os.path.join` */
+  private class OsPathJoinCallAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
+    override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+      exists(CallNode call |
+        nodeTo.asCfgNode() = call and
+        call.getFunction() = os::path::join().asCfgNode() and
+        call.getAnArg() = nodeFrom.asCfgNode()
+      )
+      // TODO: Handle pathlib (like we do for os.path.join)
+    }
+  }
 }
diff --git a/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/TestTaint.expected b/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/TestTaint.expected
@@ -139,7 +139,7 @@
 | test_string.py:149 | fail | binary_decode_encode | quopri.decodestring(..) |
 | test_string.py:158 | ok   | test_os_path_join | os.path.join(..) |
 | test_string.py:159 | ok   | test_os_path_join | os.path.join(..) |
-| test_string.py:160 | fail | test_os_path_join | os.path.join(..) |
+| test_string.py:160 | ok   | test_os_path_join | os.path.join(..) |
 | test_unpacking.py:16 | ok   | unpacking | a |
 | test_unpacking.py:16 | ok   | unpacking | b |
 | test_unpacking.py:16 | ok   | unpacking | c |

Original file line number	Diff line number	Diff line change
`@@ -130,7 +130,6 @@ predicate stringManipulation(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeT`
`130`	`130`	`// f-strings`
`131`	`131`	`nodeTo.asExpr().(Fstring).getAValue() = nodeFrom.asExpr()`
`132`	`132`	`// TODO: Handle encode/decode from base64/quopri`
`133`		`- // TODO: Handle os.path.join`
`134`	`133`	`// TODO: Handle functions in https://docs.python.org/3/library/binascii.html`
`135`	`134`	`}`
`136`	`135`