diff --git a/cpp/ql/lib/change-notes/2024-03-26-taint-inheriting-content.md b/cpp/ql/lib/change-notes/2024-03-26-taint-inheriting-content.md new file mode 100644 index 000000000000..759386e461f6 --- /dev/null +++ b/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. \ No newline at end of file diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/FlowSteps.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/FlowSteps.qll new file mode 100644 index 000000000000..673dc24b6733 --- /dev/null +++ b/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 { } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index 81fc3115f553..001e8eaac9f6 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/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) } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll index 51b893ddb238..0f69094e8d3d 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll +++ b/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 + ) } /** diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected index 54fd7cd8883c..6da494e75d75 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected +++ b/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 | | diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp index 0ba45b6f30ab..1504142bdce0 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp +++ b/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 } \ No newline at end of file diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.ql b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.ql index 56c8cd8ba684..147730278176 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.ql +++ b/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 {