From e76c07d77bb19b85557a8174acc7e4aca70411bb Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Fri, 31 Jul 2020 16:06:46 +0100 Subject: [PATCH] Temporarily taint all structs from field writes This should be either refined to just Message types, or else a macro taint step should be added conducting taint from field-write-of-argument to Marshal's result. On the read-side we're currently fine: the bytes are tainted, so the object is tainted, so the field reads are tainted. --- .../semmle/go/frameworks/Protobuf/TaintFlows.expected | 2 ++ .../semmle/go/frameworks/Protobuf/TaintFlows.ql | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/ql/test/library-tests/semmle/go/frameworks/Protobuf/TaintFlows.expected b/ql/test/library-tests/semmle/go/frameworks/Protobuf/TaintFlows.expected index 5d1527b40c0..b59461265d1 100644 --- a/ql/test/library-tests/semmle/go/frameworks/Protobuf/TaintFlows.expected +++ b/ql/test/library-tests/semmle/go/frameworks/Protobuf/TaintFlows.expected @@ -1,2 +1,4 @@ +| testDeprecatedApi.go:22:22:22:41 | call to getUntrustedString : string | testDeprecatedApi.go:26:12:26:21 | serialized | | testDeprecatedApi.go:41:25:41:43 | call to getUntrustedBytes : slice type | testDeprecatedApi.go:45:13:45:29 | selection of Description | | testDeprecatedApi.go:49:25:49:43 | call to getUntrustedBytes : slice type | testDeprecatedApi.go:53:13:53:34 | call to GetDescription | +| testDeprecatedApi.go:58:23:58:42 | call to getUntrustedString : string | testDeprecatedApi.go:65:12:65:21 | serialized | diff --git a/ql/test/library-tests/semmle/go/frameworks/Protobuf/TaintFlows.ql b/ql/test/library-tests/semmle/go/frameworks/Protobuf/TaintFlows.ql index 94a6bb709f2..7650f17f890 100644 --- a/ql/test/library-tests/semmle/go/frameworks/Protobuf/TaintFlows.ql +++ b/ql/test/library-tests/semmle/go/frameworks/Protobuf/TaintFlows.ql @@ -12,6 +12,10 @@ class SinkFunction extends Function { SinkFunction() { this.getName() = ["sinkString", "sinkBytes"] } } +predicate fieldWriteStep(DataFlow::Node pred, DataFlow::Node succ) { + any(DataFlow::Write w).writesField(succ.(DataFlow::PostUpdateNode).getPreUpdateNode(), _, pred) +} + class TestConfig extends TaintTracking::Configuration { TestConfig() { this = "testconfig" } @@ -20,6 +24,11 @@ class TestConfig extends TaintTracking::Configuration { override predicate isSink(DataFlow::Node sink) { sink = any(SinkFunction f).getACall().getAnArgument() } + + override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) { + super.isAdditionalTaintStep(fromNode, toNode) or + fieldWriteStep(fromNode, toNode) + } } from TaintTracking::Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink