diff --git a/go/ql/lib/ext/text.template.model.yml b/go/ql/lib/ext/text.template.model.yml index 669af3a8854fc..3ff0321a43c20 100644 --- a/go/ql/lib/ext/text.template.model.yml +++ b/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. diff --git a/go/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll b/go/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll index 2fcbf2d350f26..6fe005385b24b 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll +++ b/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 + ) } /** diff --git a/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll b/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll index 68ffe57f5f59d..3d66f4b96e073 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll +++ b/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`. */ diff --git a/go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll b/go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll index 85f6bb0874f50..50a632af3352f 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll +++ b/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) ) } diff --git a/go/ql/lib/semmle/go/frameworks/stdlib/TextTemplate.qll b/go/ql/lib/semmle/go/frameworks/stdlib/TextTemplate.qll index 4ef4da0583959..e67aef84f17c5 100644 --- a/go/ql/lib/semmle/go/frameworks/stdlib/TextTemplate.qll +++ b/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 + } + } } diff --git a/go/ql/test/library-tests/semmle/go/frameworks/serialization/test.expected b/go/ql/test/library-tests/semmle/go/frameworks/serialization/test.expected new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/go/ql/test/library-tests/semmle/go/frameworks/serialization/test.ql b/go/ql/test/library-tests/semmle/go/frameworks/serialization/test.ql new file mode 100644 index 0000000000000..171f2a1181b7e --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/frameworks/serialization/test.ql @@ -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 diff --git a/go/ql/test/library-tests/semmle/go/frameworks/serialization/texttemplate.go b/go/ql/test/library-tests/semmle/go/frameworks/serialization/texttemplate.go new file mode 100644 index 0000000000000..85a1372752ff8 --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/frameworks/serialization/texttemplate.go @@ -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 + +}