Merge pull request #16063 from MathiasVP/taint-inheriting-content-for-cpp

C++: Add `TaintInheritingContent`

Author: MathiasVP

cpp/ql/lib/change-notes/2024-03-26-taint-inheriting-content.md

@@ -0,0 +1,4 @@
+---
+category: feature
+---
+* Added a `TaintInheritingContent` class that can be extended to model taint flowing from a qualifier to a field.

cpp/ql/lib/semmle/code/cpp/ir/dataflow/FlowSteps.qll

@@ -0,0 +1,20 @@
+/**
+ * This file provides an abstract class that can be used to model additional
+ * object-to-field taint-flow.
+ */
+
+private import codeql.util.Unit
+private import semmle.code.cpp.dataflow.new.DataFlow
+
+/**
+ * A `Content` that should be implicitly regarded as tainted whenever an object with such `Content`
+ * is itself tainted.
+ *
+ * For example, if we had a type `struct Container { int field; }`, then by default a tainted
+ * `Container` and a `Container` with a tainted `int` stored in its `field` are distinct.
+ *
+ * If `any(DataFlow::FieldContent fc | fc.getField().hasQualifiedName("Container", "field"))` was
+ * included in this type however, then a tainted `Container` would imply that its `field` is also
+ * tainted (but not vice versa).
+ */
+abstract class TaintInheritingContent extends DataFlow::Content { }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

@@ -2301,8 +2301,8 @@ private import ContentStars
 
 /** A reference through a non-union instance field. */
 class FieldContent extends Content, TFieldContent {
-  Field f;
-  int indirectionIndex;
+  private Field f;
+  private int indirectionIndex;
 
   FieldContent() { this = TFieldContent(f, indirectionIndex) }
 
@@ -2329,9 +2329,9 @@ class FieldContent extends Content, TFieldContent {
 
 /** A reference through an instance field of a union. */
 class UnionContent extends Content, TUnionContent {
-  Union u;
-  int indirectionIndex;
-  int bytes;
+  private Union u;
+  private int indirectionIndex;
+  private int bytes;
 
   UnionContent() { this = TUnionContent(u, bytes, indirectionIndex) }
 

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll

@@ -6,6 +6,7 @@ private import semmle.code.cpp.models.interfaces.SideEffect
 private import DataFlowUtil
 private import DataFlowPrivate
 private import SsaInternals as Ssa
+private import semmle.code.cpp.ir.dataflow.FlowSteps
 
 /**
  * Holds if taint propagates from `nodeFrom` to `nodeTo` in exactly one local
@@ -37,6 +38,12 @@ predicate localAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeT
   )
   or
   any(Ssa::Indirection ind).isAdditionalTaintStep(nodeFrom, nodeTo)
+  or
+  // object->field conflation for content that is a `TaintInheritingContent`.
+  exists(DataFlow::ContentSet f |
+    readStep(nodeFrom, f, nodeTo) and
+    f.getAReadContent() instanceof TaintInheritingContent
+  )
 }
 
 /**

cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected

@@ -6676,6 +6676,7 @@ WARNING: Module TaintTracking has been deprecated and may be removed in future (
 | taint.cpp:757:7:757:10 | path | taint.cpp:759:8:759:11 | path |  |
 | taint.cpp:758:21:758:24 | ref arg path | taint.cpp:759:8:759:11 | path |  |
 | taint.cpp:759:8:759:11 | path | taint.cpp:759:7:759:11 | * ... |  |
+| taint.cpp:769:37:769:42 | call to source | taint.cpp:770:7:770:9 | obj |  |
 | vector.cpp:16:43:16:49 | source1 | vector.cpp:17:26:17:32 | source1 |  |
 | vector.cpp:16:43:16:49 | source1 | vector.cpp:31:38:31:44 | source1 |  |
 | vector.cpp:17:21:17:33 | call to vector | vector.cpp:19:14:19:14 | v |  |

cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp

@@ -757,4 +757,15 @@ void test_call_sprintf() {
 	char path[10];
 	call_sprintf_twice(path, indirect_source());
 	sink(*path); // $ ast,ir
+}
+
+struct TaintInheritingContentObject {
+	int flowFromObject;
+};
+
+TaintInheritingContentObject source(bool);
+
+void test_TaintInheritingContent() {
+	TaintInheritingContentObject obj = source(true);
+	sink(obj.flowFromObject); // $ ir MISSING: ast
 }

cpp/ql/test/library-tests/dataflow/taint-tests/taint.ql

@@ -76,6 +76,24 @@ module AstTest {
 module IRTest {
   private import semmle.code.cpp.ir.IR
   private import semmle.code.cpp.ir.dataflow.TaintTracking
+  private import semmle.code.cpp.ir.dataflow.FlowSteps
+
+  /**
+   * Object->field flow when the object is of type
+   * TaintInheritingContentObject and the field is named
+   * flowFromObject
+   */
+  class TaintInheritingContentTest extends TaintInheritingContent, DataFlow::FieldContent {
+    TaintInheritingContentTest() {
+      exists(Struct o, Field f |
+        this.getField() = f and
+        f = o.getAField() and
+        o.hasGlobalName("TaintInheritingContentObject") and
+        f.hasName("flowFromObject") and
+        this.getIndirectionIndex() = 1
+      )
+    }
+  }
 
   /** Common data flow configuration to be used by tests. */
   module TestAllocationConfig implements DataFlow::ConfigSig {