Merge pull request #17767 from github/tausbn/python-3.13-model-flow-in-replace

Python: Model `copy.replace`

Author: tausbn

python/ql/lib/change-notes/2024-10-15-models-for-copy-replace.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+
+- Added partial support for the `copy.replace` method, [added](https://docs.python.org/3.13/library/copy.html#copy.replace) in Python 3.13.

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

@@ -687,16 +687,23 @@ newtype TContent =
 class Content extends TContent {
   /** Gets a textual representation of this element. */
   string toString() { result = "Content" }
+
+  /** Gets the Models-as-Data representation of this content (if any). */
+  string getMaDRepresentation() { none() }
 }
 
 /** An element of a list. */
 class ListElementContent extends TListElementContent, Content {
   override string toString() { result = "List element" }
+
+  override string getMaDRepresentation() { result = "ListElement" }
 }
 
 /** An element of a set. */
 class SetElementContent extends TSetElementContent, Content {
   override string toString() { result = "Set element" }
+
+  override string getMaDRepresentation() { result = "SetElement" }
 }
 
 /** An element of a tuple at a specific index. */
@@ -709,6 +716,8 @@ class TupleElementContent extends TTupleElementContent, Content {
   int getIndex() { result = index }
 
   override string toString() { result = "Tuple element at index " + index.toString() }
+
+  override string getMaDRepresentation() { result = "TupleElement[" + index + "]" }
 }
 
 /** An element of a dictionary under a specific key. */
@@ -721,11 +730,15 @@ class DictionaryElementContent extends TDictionaryElementContent, Content {
   string getKey() { result = key }
 
   override string toString() { result = "Dictionary element at key " + key }
+
+  override string getMaDRepresentation() { result = "DictionaryElement[" + key + "]" }
 }
 
 /** An element of a dictionary under any key. */
 class DictionaryElementAnyContent extends TDictionaryElementAnyContent, Content {
   override string toString() { result = "Any dictionary element" }
+
+  override string getMaDRepresentation() { result = "DictionaryElementAny" }
 }
 
 /** An object attribute. */
@@ -738,6 +751,8 @@ class AttributeContent extends TAttributeContent, Content {
   string getAttribute() { result = attr }
 
   override string toString() { result = "Attribute " + attr }
+
+  override string getMaDRepresentation() { result = "Attribute[" + attr + "]" }
 }
 
 /** A captured variable. */
@@ -750,6 +765,8 @@ class CapturedVariableContent extends Content, TCapturedVariableContent {
   VariableCapture::CapturedVariable getVariable() { result = v }
 
   override string toString() { result = "captured " + v }
+
+  override string getMaDRepresentation() { none() }
 }
 
 /**

python/ql/lib/semmle/python/frameworks/Stdlib.model.yml

@@ -45,7 +45,7 @@ extensions:
       # See https://docs.python.org/3/library/contextlib.html#contextlib.ExitStack
       - ["contextlib.ExitStack", "Member[enter_context]", "Argument[0,cm:]", "ReturnValue", "taint"]
       # See https://docs.python.org/3/library/copy.html#copy.deepcopy
-      - ["copy", "Member[copy,deepcopy]", "Argument[0,x:]", "ReturnValue", "value"]
+      - ["copy", "Member[copy,deepcopy,replace]", "Argument[0,x:]", "ReturnValue", "value"]
       # See
       #  - https://docs.python.org/3/library/ctypes.html#ctypes.create_string_buffer
       #  - https://docs.python.org/3/library/ctypes.html#ctypes.create_unicode_buffer

python/ql/lib/semmle/python/frameworks/Stdlib.qll

@@ -4537,21 +4537,9 @@ module StdlibPrivate {
     override DataFlow::ArgumentNode getACallback() { none() }
 
     override predicate propagatesFlow(string input, string output, boolean preservesValue) {
-      exists(string content |
-        content = "ListElement"
-        or
-        content = "SetElement"
-        or
-        exists(DataFlow::TupleElementContent tc, int i | i = tc.getIndex() |
-          content = "TupleElement[" + i.toString() + "]"
-        )
-        or
-        exists(DataFlow::DictionaryElementContent dc, string key | key = dc.getKey() |
-          content = "DictionaryElement[" + key + "]"
-        )
-      |
-        input = "Argument[self]." + content and
-        output = "ReturnValue." + content and
+      exists(DataFlow::Content c |
+        input = "Argument[self]." + c.getMaDRepresentation() and
+        output = "ReturnValue." + c.getMaDRepresentation() and
         preservesValue = true
       )
       or
@@ -4561,6 +4549,32 @@ module StdlibPrivate {
     }
   }
 
+  /** A flow summary for `copy.replace`. */
+  class ReplaceSummary extends SummarizedCallable {
+    ReplaceSummary() { this = "copy.replace" }
+
+    override DataFlow::CallCfgNode getACall() {
+      result = API::moduleImport("copy").getMember("replace").getACall()
+    }
+
+    override DataFlow::ArgumentNode getACallback() {
+      result = API::moduleImport("copy").getMember("replace").getAValueReachableFromSource()
+    }
+
+    override predicate propagatesFlow(string input, string output, boolean preservesValue) {
+      exists(CallNode c, string name, ControlFlowNode n, DataFlow::AttributeContent ac |
+        c.getFunction().(NameNode).getId() = "replace" or
+        c.getFunction().(AttrNode).getName() = "replace"
+      |
+        n = c.getArgByName(name) and
+        ac.getAttribute() = name and
+        input = "Argument[" + name + ":]" and
+        output = "ReturnValue." + ac.getMaDRepresentation() and
+        preservesValue = true
+      )
+    }
+  }
+
   /**
    * A flow summary for `pop` either for list or set.
    * This ignores the index if given, since content is

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

@@ -166,6 +166,34 @@ def test_copy_2():
         copy.deepcopy(TAINTED_LIST), # $ tainted
     )
 
+def test_replace():
+    from copy import replace
+
+    class C:
+        def __init__(self, always_tainted, tainted_to_safe, safe_to_tainted, always_safe):
+            self.always_tainted = always_tainted
+            self.tainted_to_safe = tainted_to_safe
+            self.safe_to_tainted = safe_to_tainted
+            self.always_safe = always_safe
+
+    c = C(always_tainted=TAINTED_STRING,
+          tainted_to_safe=TAINTED_STRING,
+          safe_to_tainted=NOT_TAINTED,
+          always_safe=NOT_TAINTED)
+
+    d = replace(c, tainted_to_safe=NOT_TAINTED, safe_to_tainted=TAINTED_STRING)
+
+    ensure_tainted(d.always_tainted) # $ tainted
+    ensure_tainted(d.safe_to_tainted) # $ tainted
+    ensure_not_tainted(d.always_safe)
+
+    # Currently, we have no way of stopping the value in the tainted_to_safe field (which gets
+    # overwritten) from flowing through the replace call, which means we get a spurious result.
+
+    ensure_not_tainted(d.tainted_to_safe) # $ SPURIOUS: tainted
+
+
+
 
 def list_index_assign():
     tainted_string = TAINTED_STRING