Python: Remove imprecise container steps

- remove `tupleStoreStep` and `dictStoreStep` from `containerStep`
   These are imprecise compared to the content being precise.
- add implicit reads to recover taint at sinks
- add implicit read steps for decoders
  to supplement the `AdditionalTaintStep`
  that now only covers when the full container is tainted.

Author: yoff

python/ql/consistency-queries/DataFlowConsistency.ql

@@ -34,6 +34,8 @@ private module Input implements InputSig<Location, PythonDataFlow> {
     // parameter, but dataflow-consistency queries should _not_ complain about there not
     // being a post-update node for the synthetic `**kwargs` parameter.
     n instanceof SynthDictSplatParameterNode
+    or
+    Private::Conversions::readStep(n, _, _)
   }
 
   predicate uniqueParameterNodePositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) {

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

@@ -928,6 +928,8 @@ predicate readStep(Node nodeFrom, ContentSet c, Node nodeTo) {
   synthDictSplatParameterNodeReadStep(nodeFrom, c, nodeTo)
   or
   VariableCapture::readStep(nodeFrom, c, nodeTo)
+  or
+  Conversions::readStep(nodeFrom, c, nodeTo)
 }
 
 /** Data flows from a sequence to a subscript of the sequence. */
@@ -983,6 +985,40 @@ predicate attributeReadStep(Node nodeFrom, AttributeContent c, AttrRead nodeTo)
   nodeTo.accesses(nodeFrom, c.getAttribute())
 }
 
+module Conversions {
+  private import semmle.python.Concepts
+
+  predicate decoderReadStep(Node nodeFrom, ContentSet c, Node nodeTo) {
+    exists(Decoding decoding |
+      nodeFrom = decoding.getAnInput() and
+      nodeTo = decoding.getOutput()
+    ) and
+    (
+      c instanceof TupleElementContent
+      or
+      c instanceof DictionaryElementContent
+    )
+  }
+
+  predicate encoderReadStep(Node nodeFrom, ContentSet c, Node nodeTo) {
+    exists(Encoding encoding |
+      nodeFrom = encoding.getAnInput() and
+      nodeTo = encoding.getOutput()
+    ) and
+    (
+      c instanceof TupleElementContent
+      or
+      c instanceof DictionaryElementContent
+    )
+  }
+
+  predicate readStep(Node nodeFrom, ContentSet c, Node nodeTo) {
+    decoderReadStep(nodeFrom, c, nodeTo)
+    or
+    encoderReadStep(nodeFrom, c, nodeTo)
+  }
+}
+
 /**
  * Holds if values stored inside content `c` are cleared at node `n`. For example,
  * any value stored inside `f` is cleared at the pre-update node associated with `x`

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

@@ -16,7 +16,16 @@ predicate defaultTaintSanitizer(DataFlow::Node node) { none() }
  * of `c` at sinks and inputs to additional taint steps.
  */
 bindingset[node]
-predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::ContentSet c) { none() }
+predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::ContentSet c) {
+  // We allow implicit reads of precise content
+  // imprecise content has already bubled up.
+  exists(node) and
+  (
+    c instanceof DataFlow::TupleElementContent
+    or
+    c instanceof DataFlow::DictionaryElementContent
+  )
+}
 
 private module Cached {
   /**
@@ -178,10 +187,6 @@ predicate containerStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
   or
   DataFlowPrivate::setStoreStep(nodeFrom, _, nodeTo)
   or
-  DataFlowPrivate::tupleStoreStep(nodeFrom, _, nodeTo)
-  or
-  DataFlowPrivate::dictStoreStep(nodeFrom, _, nodeTo)
-  or
   // comprehension, so there is taint-flow from `x` in `[x for x in xs]` to the
   // resulting list of the list-comprehension.
   //

python/ql/test/library-tests/dataflow/sensitive-data/test.py

@@ -131,6 +131,5 @@ def call_wrapper(func):
 print(password) # $ SensitiveUse=password
 _config = {"sleep_timer": 5, "mysql_password": password}
 
-# since we have taint-step from store of `password`, we will consider any item in the
-# dictionary to be a password :(
-print(_config["sleep_timer"]) # $ SPURIOUS: SensitiveUse=password
+# since we have precise dictionary content, other items of the config are not tainted
+print(_config["sleep_timer"])

python/ql/test/library-tests/dataflow/tainttracking/defaultAdditionalTaintStep-py3/test_string.py

@@ -17,7 +17,7 @@ def str_methods():
         ts.casefold(), # $ tainted
 
         ts.format_map({}), # $ tainted
-        "{unsafe}".format_map({"unsafe": ts}), # $ tainted
+        "{unsafe}".format_map({"unsafe": ts}), # $ MISSING: tainted
     )
 
 

python/ql/test/library-tests/dataflow/tainttracking/defaultAdditionalTaintStep/test_collections.py

@@ -29,10 +29,10 @@ def test_construction():
 
     ensure_tainted(
         list(tainted_list), # $ tainted
-        list(tainted_tuple), # $ tainted
+        list(tainted_tuple), # $ MISSING: tainted
         list(tainted_set), # $ tainted
-        list(tainted_dict.values()), # $ tainted
-        list(tainted_dict.items()), # $ tainted
+        list(tainted_dict.values()), # $ MISSING: tainted
+        list(tainted_dict.items()), # $ MISSING: tainted
 
         tuple(tainted_list), # $ tainted
         set(tainted_list), # $ tainted

python/ql/test/library-tests/dataflow/tainttracking/defaultAdditionalTaintStep/test_string.py

@@ -115,7 +115,7 @@ def percent_fmt():
     ensure_tainted(
         tainted_fmt % (1, 2), # $ tainted
         "%s foo bar" % ts, # $ tainted
-        "%s %s %s" % (1, 2, ts), # $ tainted
+        "%s %s %s" % (1, 2, ts), # $ MISSING: tainted
     )
 
 

python/ql/test/library-tests/dataflow/tainttracking/defaultAdditionalTaintStep/test_unpacking.py

@@ -53,7 +53,7 @@ def contrived_1():
 
     (a, b, c), (d, e, f) = tainted_list, no_taint_list
     ensure_tainted(a, b, c) # $ tainted
-    ensure_not_tainted(d, e, f) # $ SPURIOUS: tainted
+    ensure_not_tainted(d, e, f)
 
 
 def contrived_2():

python/ql/test/library-tests/frameworks/stdlib/test_re.py

@@ -74,9 +74,9 @@
 )
 
 ensure_not_tainted(
-    re.subn(pat, repl="safe", string=ts),
     re.subn(pat, repl="safe", string=ts)[1], # // the number of substitutions made
 )
 ensure_tainted(
+    re.subn(pat, repl="safe", string=ts), # $ tainted // implicit read at sink
     re.subn(pat, repl="safe", string=ts)[0], # $ tainted // the string
 )

python/ql/test/library-tests/frameworks/tornado/taint_test.py

@@ -63,7 +63,7 @@ def get(self, name = "World!", number="0", foo="foo"):  # $ requestHandler route
             request.headers["header-name"], # $ tainted
             request.headers.get_list("header-name"), # $ tainted
             request.headers.get_all(), # $ tainted
-            [(k, v) for (k, v) in request.headers.get_all()], # $ tainted
+            [(k, v) for (k, v) in request.headers.get_all()], # $ MISSING: tainted
 
             # Dict[str, http.cookies.Morsel]
             request.cookies, # $ tainted