template/text.Template execution methods: support reading arbitrary content

Author: smowton

go/ql/lib/ext/text.template.model.yml

@@ -7,5 +7,5 @@ extensions:
       - ["text/template", "", False, "HTMLEscapeString", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["text/template", "", False, "JSEscape", "", "", "Argument[1]", "Argument[0]", "taint", "manual"]
       - ["text/template", "", False, "JSEscapeString", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
-      - ["text/template", "Template", True, "Execute", "", "", "Argument[1]", "Argument[0]", "taint", "manual"]
-      - ["text/template", "Template", True, "ExecuteTemplate", "", "", "Argument[2]", "Argument[0]", "taint", "manual"]
+#      - ["text/template", "Template", True, "Execute", "", "", "Argument[1]", "Argument[0]", "taint", "manual"] # Implemented in QL to provide an arbitrary content read from the input.
+#      - ["text/template", "Template", True, "ExecuteTemplate", "", "", "Argument[2]", "Argument[0]", "taint", "manual"] # Implemented in QL to provide an arbitrary content read from the input.

go/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll

@@ -165,6 +165,53 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
   containerStoreStep(node1, node2, c)
 }
 
+/**
+ * Gets a `DataFlow::ContentSet` containing a single `Content` appropriate
+ * for reading a field, element, map value or channel message of type `containerType`.
+ */
+DataFlow::ContentSet getContentForType(Type containerType) {
+  containerType instanceof ArrayType and
+  result instanceof DataFlow::ArrayContent
+  or
+  containerType instanceof SliceType and
+  result instanceof DataFlow::ArrayContent
+  or
+  containerType instanceof ChanType and
+  result instanceof DataFlow::CollectionContent
+  or
+  containerType instanceof MapType and
+  result instanceof DataFlow::MapValueContent
+  or
+  result.(DataFlow::PointerContent).getPointerType() = containerType
+  or
+  exists(Field f | f = containerType.(StructType).getField(_) |
+    result.(DataFlow::FieldContent).getField() = f
+  )
+}
+
+/**
+ * Gets the type of an array/slice element, channel value, map value,
+ * pointer base type or named-type underlying type relating to `containerType`.
+ */
+Type getElementType(Type containerType) {
+  result = containerType.(ArrayType).getElementType() or
+  result = containerType.(SliceType).getElementType() or
+  result = containerType.(ChanType).getElementType() or
+  result = containerType.(MapType).getValueType() or
+  result = containerType.(PointerType).getPointerType() or
+  result = containerType.(NamedType).getUnderlyingType()
+}
+
+/**
+ * Gets the type of an array/slice element, channel value, map value,
+ * pointer base type, named-type underlying type or struct field type
+ * relating to `containerType`.
+ */
+Type getAnElementOrFieldType(Type containerType) {
+  result = getElementType(containerType) or
+  result = containerType.(StructType).getField(_).getType()
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` via a read of `c`.
  * Thus, `node1` references an object with a content `c` whose value ends up in
@@ -184,6 +231,14 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
     node2.(FlowSummaryNode).getSummaryNode())
   or
   containerReadStep(node1, node2, c)
+  or
+  exists(Type containerType |
+    any(ImplicitFieldReadNode ifrn).shouldImplicitlyReadAllFields(node1) and
+    getAnElementOrFieldType*(node1.getType()) = containerType
+  |
+    c = getContentForType(containerType) and
+    node1 = node2
+  )
 }
 
 /**

go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll

@@ -7,6 +7,7 @@ private import semmle.go.dataflow.FunctionInputsAndOutputs
 private import semmle.go.dataflow.ExternalFlow
 private import DataFlowPrivate
 private import FlowSummaryImpl as FlowSummaryImpl
+private import codeql.util.Unit
 import DataFlowNodes::Public
 
 /**
@@ -50,6 +51,18 @@ abstract class FunctionModel extends Function {
   }
 }
 
+/**
+ * A unit class for adding nodes that should implicitly read from all nested content.
+ *
+ * For example, this might be appopriate for the argument to a method that serializes a struct.
+ */
+class ImplicitFieldReadNode extends Unit {
+  /**
+   * Holds if the node `n` should implicitly read from all nested content in a taint-tracking context.
+   */
+  abstract predicate shouldImplicitlyReadAllFields(DataFlow::Node n);
+}
+
 /**
  * Gets the `Node` corresponding to `insn`.
  */

go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll

@@ -34,14 +34,6 @@ predicate localTaintStep(DataFlow::Node src, DataFlow::Node sink) {
   FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(src, sink, _)
 }
 
-private Type getElementType(Type containerType) {
-  result = containerType.(ArrayType).getElementType() or
-  result = containerType.(SliceType).getElementType() or
-  result = containerType.(ChanType).getElementType() or
-  result = containerType.(MapType).getValueType() or
-  result = containerType.(PointerType).getPointerType()
-}
-
 /**
  * Holds if default `TaintTracking::Configuration`s should allow implicit reads
  * of `c` at sinks and inputs to additional taint steps.
@@ -50,21 +42,9 @@ bindingset[node]
 predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::ContentSet c) {
   exists(Type containerType |
     node instanceof DataFlow::ArgumentNode and
-    getElementType*(node.getType()) = containerType
+    DataFlowPrivate::getElementType*(node.getType()) = containerType
   |
-    containerType instanceof ArrayType and
-    c instanceof DataFlow::ArrayContent
-    or
-    containerType instanceof SliceType and
-    c instanceof DataFlow::ArrayContent
-    or
-    containerType instanceof ChanType and
-    c instanceof DataFlow::CollectionContent
-    or
-    containerType instanceof MapType and
-    c instanceof DataFlow::MapValueContent
-    or
-    c.(DataFlow::PointerContent).getPointerType() = containerType
+    c = DataFlowPrivate::getContentForType(containerType)
   )
 }
 

go/ql/lib/semmle/go/frameworks/stdlib/TextTemplate.qll

@@ -67,4 +67,44 @@ module TextTemplate {
       input = inp and output = outp
     }
   }
+
+  private class ExecuteTemplateMethod extends Method {
+    string name;
+    int inputArg;
+
+    ExecuteTemplateMethod() {
+      this.hasQualifiedName("text/template", "Template", name) and
+      (
+        name = "Execute" and inputArg = 1
+        or
+        name = "ExecuteTemplate" and inputArg = 2
+      )
+    }
+
+    int getInputArgIdx() { result = inputArg }
+  }
+
+  private class ExecuteTemplateFieldReader extends DataFlow::ImplicitFieldReadNode {
+    override predicate shouldImplicitlyReadAllFields(DataFlow::Node n) {
+      exists(ExecuteTemplateMethod m, DataFlow::MethodCallNode cn |
+        cn.getACalleeIncludingExternals().asFunction() = m and
+        n = cn.getArgument(m.getInputArgIdx())
+      )
+    }
+  }
+
+  private class ExecuteTemplateFunctionModels extends TaintTracking::FunctionModel,
+    ExecuteTemplateMethod
+  {
+    FunctionInput inp;
+    FunctionOutput outp;
+
+    ExecuteTemplateFunctionModels() {
+      inp.isParameter(this.getInputArgIdx()) and outp.isParameter(0)
+    }
+
+    override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+      input = inp and output = outp
+    }
+  }
 }

go/ql/test/library-tests/semmle/go/frameworks/serialization/test.expected

@@ -0,0 +1,9 @@
+import go
+import TestUtilities.InlineFlowTest
+
+string getArgString(DataFlow::Node src, DataFlow::Node sink) {
+  exists(sink) and
+  result = src.(DataFlow::CallNode).getArgument(0).getExactValue()
+}
+
+import TaintFlowTestArgString<DefaultFlowConfig, getArgString/2>

go/ql/test/library-tests/semmle/go/frameworks/serialization/test.ql

@@ -0,0 +1,50 @@
+package xyz
+
+import (
+	"bytes"
+	"text/template"
+)
+
+type Inner1 struct {
+	Data string
+}
+
+type Inner2 struct {
+	Data string
+}
+
+type Inner3 struct {
+	Data string
+}
+
+type Outer struct {
+	SliceField []Inner1
+	PtrField   *Inner2
+	MapField   map[string]Inner3
+}
+
+func source(n int) string { return "dummy" }
+func sink(arg any)        {}
+
+func test() {
+
+	source1 := source(1)
+	source2 := source(2)
+	source3 := source(3)
+
+	toSerialize := Outer{[]Inner1{{source1}}, &Inner2{source2}, map[string]Inner3{"key": {source3}}}
+	buff1 := new(bytes.Buffer)
+	buff2 := new(bytes.Buffer)
+	bytes1 := make([]byte, 10)
+	bytes2 := make([]byte, 10)
+
+	tmpl, _ := template.New("test").Parse("Template text goes here (irrelevant for test)")
+	tmpl.ExecuteTemplate(buff1, "test", toSerialize)
+	buff1.Read(bytes1)
+	sink(bytes1) // $ hasTaintFlow=1 hasTaintFlow=2 hasTaintFlow=3
+
+	tmpl.Execute(buff2, toSerialize)
+	buff2.Read(bytes2)
+	sink(bytes2) // $ hasTaintFlow=1 hasTaintFlow=2 hasTaintFlow=3
+
+}