From a832342ecb641b558b9ccaeaf73ec8d68bc91e69 Mon Sep 17 00:00:00 2001
From: Chris Smowton <smowton@github.com>
Date: Wed, 12 Aug 2020 12:02:52 +0100
Subject: [PATCH] Add test for unmarshalling submessages

---
 .../Protobuf/FunctionModel.expected           |  1 +
 .../frameworks/Protobuf/TaintFlows.expected   |  1 +
 .../frameworks/Protobuf/testDeprecatedApi.go  | 22 +++++++++++++------
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/ql/test/library-tests/semmle/go/frameworks/Protobuf/FunctionModel.expected b/ql/test/library-tests/semmle/go/frameworks/Protobuf/FunctionModel.expected
index caa43bec163..9d50359f14e 100644
--- a/ql/test/library-tests/semmle/go/frameworks/Protobuf/FunctionModel.expected
+++ b/ql/test/library-tests/semmle/go/frameworks/Protobuf/FunctionModel.expected
@@ -16,6 +16,7 @@
 | testDeprecatedApi.go:84:24:84:35 | selection of Alerts | testDeprecatedApi.go:84:17:84:43 | call to append |
 | testDeprecatedApi.go:84:38:84:42 | alert | testDeprecatedApi.go:84:17:84:43 | call to append |
 | testDeprecatedApi.go:87:33:87:37 | query | testDeprecatedApi.go:87:2:87:38 | ... := ...[0] |
+| testDeprecatedApi.go:95:18:95:36 | untrustedSerialized | testDeprecatedApi.go:94:2:94:6 | definition of query |
 | testModernApi.go:13:33:13:37 | query | testModernApi.go:13:2:13:38 | ... := ...[0] |
 | testModernApi.go:22:28:22:32 | query | testModernApi.go:22:16:22:33 | call to Clone |
 | testModernApi.go:24:33:24:42 | queryClone | testModernApi.go:24:2:24:43 | ... := ...[0] |
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 0adc3b6fcc7..0289e5757e8 100644
--- a/ql/test/library-tests/semmle/go/frameworks/Protobuf/TaintFlows.expected
+++ b/ql/test/library-tests/semmle/go/frameworks/Protobuf/TaintFlows.expected
@@ -5,6 +5,7 @@
 | testDeprecatedApi.go:58:23:58:42 | call to getUntrustedString : string | testDeprecatedApi.go:65:12:65:21 | serialized |
 | testDeprecatedApi.go:70:14:70:33 | call to getUntrustedString : string | testDeprecatedApi.go:77:12:77:21 | serialized |
 | testDeprecatedApi.go:85:24:85:43 | call to getUntrustedString : string | testDeprecatedApi.go:89:12:89:21 | serialized |
+| testDeprecatedApi.go:93:25:93:43 | call to getUntrustedBytes : slice type | testDeprecatedApi.go:97:13:97:31 | selection of Msg |
 | testModernApi.go:11:22:11:41 | call to getUntrustedString : string | testModernApi.go:15:12:15:21 | serialized |
 | testModernApi.go:20:22:20:41 | call to getUntrustedString : string | testModernApi.go:26:12:26:21 | serialized |
 | testModernApi.go:30:25:30:43 | call to getUntrustedBytes : slice type | testModernApi.go:34:13:34:29 | selection of Description |
diff --git a/ql/test/library-tests/semmle/go/frameworks/Protobuf/testDeprecatedApi.go b/ql/test/library-tests/semmle/go/frameworks/Protobuf/testDeprecatedApi.go
index 3ac8a54e9f8..a933eff62c1 100644
--- a/ql/test/library-tests/semmle/go/frameworks/Protobuf/testDeprecatedApi.go
+++ b/ql/test/library-tests/semmle/go/frameworks/Protobuf/testDeprecatedApi.go
@@ -23,7 +23,7 @@ func testMarshal() {
 
 	serialized, _ := proto.Marshal(query)
 
-	sinkBytes(serialized)
+	sinkBytes(serialized) // BAD
 }
 
 func testCloneThenMarshal() {
@@ -34,7 +34,7 @@ func testCloneThenMarshal() {
 
 	serialized, _ := proto.Marshal(queryClone)
 
-	sinkBytes(serialized)
+	sinkBytes(serialized) // BAD
 }
 
 func testUnmarshalFieldAccess() {
@@ -42,7 +42,7 @@ func testUnmarshalFieldAccess() {
 	query := &query.Query{}
 	proto.Unmarshal(untrustedSerialized, query)
 
-	sinkString(query.Description)
+	sinkString(query.Description) // BAD
 }
 
 func testUnmarshalGetter() {
@@ -50,7 +50,7 @@ func testUnmarshalGetter() {
 	query := &query.Query{}
 	proto.Unmarshal(untrustedSerialized, query)
 
-	sinkString(query.GetDescription())
+	sinkString(query.GetDescription()) // BAD
 }
 
 func testMergeThenMarshal() {
@@ -62,7 +62,7 @@ func testMergeThenMarshal() {
 
 	serialized, _ := proto.Marshal(query2)
 
-	sinkBytes(serialized)
+	sinkBytes(serialized) // BAD
 }
 
 func testTaintedSubmessage() {
@@ -74,7 +74,7 @@ func testTaintedSubmessage() {
 
 	serialized, _ := proto.Marshal(query)
 
-	sinkBytes(serialized)
+	sinkBytes(serialized) // BAD
 }
 
 func testTaintedSubmessageInPlace() {
@@ -86,5 +86,13 @@ func testTaintedSubmessageInPlace() {
 
 	serialized, _ := proto.Marshal(query)
 
-	sinkBytes(serialized)
+	sinkBytes(serialized) // BAD
+}
+
+func testUnmarshalTaintedSubmessage() {
+	untrustedSerialized := getUntrustedBytes()
+	query := &query.Query{}
+	proto.Unmarshal(untrustedSerialized, query)
+
+	sinkString(query.Alerts[0].Msg) // BAD
 }