From ff848a502a683f84147285d6183d5c7c7488538c Mon Sep 17 00:00:00 2001 From: Slavomir Date: Mon, 12 Apr 2021 14:25:21 +0200 Subject: [PATCH 01/10] ResponseBody: Use .getAPredecessor*().getStringValue() instead of just .getStringValue() --- ql/src/experimental/frameworks/CleverGo.qll | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ql/src/experimental/frameworks/CleverGo.qll b/ql/src/experimental/frameworks/CleverGo.qll index 841ae4cd5c2..08e8219dc7b 100644 --- a/ql/src/experimental/frameworks/CleverGo.qll +++ b/ql/src/experimental/frameworks/CleverGo.qll @@ -293,12 +293,12 @@ private module CleverGo { // signature: func (*Context).Blob(code int, contentType string, bs []byte) (err error) methodName = "Blob" and this = bodySetterCall.getArgument(2) and - contentType = bodySetterCall.getArgument(1).getStringValue() + contentType = bodySetterCall.getArgument(1).getAPredecessor*().getStringValue() or // signature: func (*Context).Emit(code int, contentType string, body string) (err error) methodName = "Emit" and this = bodySetterCall.getArgument(2) and - contentType = bodySetterCall.getArgument(1).getStringValue() + contentType = bodySetterCall.getArgument(1).getAPredecessor*().getStringValue() ) or // Two calls, one to set the response body and one to set the content-type. @@ -325,7 +325,7 @@ private module CleverGo { | // signature: func (*Context).SetContentType(v string) methodName = "SetContentType" and - contentType = contentTypeSetterCall.getArgument(0).getStringValue() + contentType = contentTypeSetterCall.getArgument(0).getAPredecessor*().getStringValue() ) or // Receiver type: Context From 78b403f42eb4d849764fa5b5fdb611fe0758596f Mon Sep 17 00:00:00 2001 From: Slavomir Date: Mon, 12 Apr 2021 23:37:22 +0200 Subject: [PATCH 02/10] Stub alternative HTTP::ResponseBody model implementation --- ql/src/experimental/frameworks/CleverGo.qll | 383 +++++++++++------- .../frameworks/CleverGo/HttpResponseBody.go | 3 +- 2 files changed, 234 insertions(+), 152 deletions(-) diff --git a/ql/src/experimental/frameworks/CleverGo.qll b/ql/src/experimental/frameworks/CleverGo.qll index 08e8219dc7b..fc9e3e08bdb 100644 --- a/ql/src/experimental/frameworks/CleverGo.qll +++ b/ql/src/experimental/frameworks/CleverGo.qll @@ -198,168 +198,249 @@ private module CleverGo { /** * Models HTTP ResponseBody. */ - private class HttpResponseBody extends HTTP::ResponseBody::Range { - string package; - DataFlow::CallNode bodySetterCall; - string contentType; + private class HttpResponseBodyContentTypeString extends HTTP::ResponseBody::Range { + string contentTypeString; - HttpResponseBody() { - // HTTP ResponseBody models for package: clevergo.tech/clevergo@v0.5.2 - package = packagePath() and - ( - // One call sets both body and content-type (which is implicit in the func name). - // Receiver type: Context - exists(string methodName, Method m | - m.hasQualifiedName(package, "Context", methodName) and - bodySetterCall = m.getACall() - | - // signature: func (*Context).Error(code int, msg string) error - methodName = "Error" and - this = bodySetterCall.getArgument(1) and - contentType = "text/plain" - or - // signature: func (*Context).HTML(code int, html string) error - methodName = "HTML" and - this = bodySetterCall.getArgument(1) and - contentType = "text/html" - or - // signature: func (*Context).HTMLBlob(code int, bs []byte) error - methodName = "HTMLBlob" and - this = bodySetterCall.getArgument(1) and - contentType = "text/html" - or - // signature: func (*Context).JSON(code int, data interface{}) error - methodName = "JSON" and - this = bodySetterCall.getArgument(1) and - contentType = "application/json" - or - // signature: func (*Context).JSONBlob(code int, bs []byte) error - methodName = "JSONBlob" and - this = bodySetterCall.getArgument(1) and - contentType = "application/json" - or - // signature: func (*Context).JSONP(code int, data interface{}) error - methodName = "JSONP" and - this = bodySetterCall.getArgument(1) and - contentType = "application/javascript" - or - // signature: func (*Context).JSONPBlob(code int, bs []byte) error - methodName = "JSONPBlob" and - this = bodySetterCall.getArgument(1) and - contentType = "application/javascript" - or - // signature: func (*Context).JSONPCallback(code int, callback string, data interface{}) error - methodName = "JSONPCallback" and - this = bodySetterCall.getArgument(2) and - contentType = "application/javascript" - or - // signature: func (*Context).JSONPCallbackBlob(code int, callback string, bs []byte) (err error) - methodName = "JSONPCallbackBlob" and - this = bodySetterCall.getArgument(2) and - contentType = "application/javascript" - or - // signature: func (*Context).String(code int, s string) error - methodName = "String" and - this = bodySetterCall.getArgument(1) and - contentType = "text/plain" - or - // signature: func (*Context).StringBlob(code int, bs []byte) error - methodName = "StringBlob" and - this = bodySetterCall.getArgument(1) and - contentType = "text/plain" - or - // signature: func (*Context).Stringf(code int, format string, a ...interface{}) error - methodName = "Stringf" and - this = bodySetterCall.getArgument([1, any(int i | i >= 2)]) and - contentType = "text/plain" - or - // signature: func (*Context).XML(code int, data interface{}) error - methodName = "XML" and - this = bodySetterCall.getArgument(1) and - contentType = "text/xml" - or - // signature: func (*Context).XMLBlob(code int, bs []byte) error - methodName = "XMLBlob" and - this = bodySetterCall.getArgument(1) and - contentType = "text/xml" - ) + HttpResponseBodyContentTypeString() { + exists(string package, string receiverName | + holdsBodyAndContentTypeString(package, receiverName, this, contentTypeString) or - // One call sets both body and content-type (both are parameters in the func call). - // Receiver type: Context - exists(string methodName, Method m | - m.hasQualifiedName(package, "Context", methodName) and - bodySetterCall = m.getACall() + exists(DataFlow::CallNode bodySetterCall, DataFlow::CallNode contentTypeSetterCall | + holdsBodyOnly(package, receiverName, bodySetterCall, this) and + holdsContentTypeString(package, receiverName, contentTypeSetterCall, contentTypeString) | - // signature: func (*Context).Blob(code int, contentType string, bs []byte) (err error) - methodName = "Blob" and - this = bodySetterCall.getArgument(2) and - contentType = bodySetterCall.getArgument(1).getAPredecessor*().getStringValue() - or - // signature: func (*Context).Emit(code int, contentType string, body string) (err error) - methodName = "Emit" and - this = bodySetterCall.getArgument(2) and - contentType = bodySetterCall.getArgument(1).getAPredecessor*().getStringValue() - ) - or - // Two calls, one to set the response body and one to set the content-type. - // Receiver type: Context - exists(string methodName, Method m | - m.hasQualifiedName(package, "Context", methodName) and - bodySetterCall = m.getACall() - | - // signature: func (*Context).Write(data []byte) (int, error) - methodName = "Write" and - this = bodySetterCall.getArgument(0) - or - // signature: func (*Context).WriteString(data string) (int, error) - methodName = "WriteString" and - this = bodySetterCall.getArgument(0) - ) and - ( - // Receiver type: Context - exists(string methodName, Method m, DataFlow::CallNode contentTypeSetterCall | - m.hasQualifiedName(package, "Context", methodName) and - contentTypeSetterCall = m.getACall() and - contentTypeSetterCall.getReceiver().getAPredecessor*() = - bodySetterCall.getReceiver().getAPredecessor*() - | - // signature: func (*Context).SetContentType(v string) - methodName = "SetContentType" and - contentType = contentTypeSetterCall.getArgument(0).getAPredecessor*().getStringValue() - ) - or - // Receiver type: Context - exists(string methodName, Method m, DataFlow::CallNode contentTypeSetterCall | - m.hasQualifiedName(package, "Context", methodName) and - contentTypeSetterCall = m.getACall() and - contentTypeSetterCall.getReceiver().getAPredecessor*() = - bodySetterCall.getReceiver().getAPredecessor*() - | - // signature: func (*Context).SetContentTypeHTML() - methodName = "SetContentTypeHTML" and - contentType = "text/html" - or - // signature: func (*Context).SetContentTypeJSON() - methodName = "SetContentTypeJSON" and - contentType = "application/json" - or - // signature: func (*Context).SetContentTypeText() - methodName = "SetContentTypeText" and - contentType = "text/plain" - or - // signature: func (*Context).SetContentTypeXML() - methodName = "SetContentTypeXML" and - contentType = "text/xml" - ) + contentTypeSetterCall.getReceiver().getAPredecessor*() = + bodySetterCall.getReceiver().getAPredecessor*() ) ) } - override string getAContentType() { result = contentType } + override string getAContentType() { result = contentTypeString } override HTTP::ResponseWriter getResponseWriter() { none() } } + /** + * Models HTTP ResponseBody. + */ + private class HttpResponseBodyContentTypeNode extends HTTP::ResponseBody::Range { + DataFlow::Node contentTypeNode; + + HttpResponseBodyContentTypeNode() { + exists(string package, string receiverName | + holdsBodyAndContentTypeNode(package, receiverName, this, contentTypeNode) + or + exists(DataFlow::CallNode bodySetterCall, DataFlow::CallNode contentTypeSetterCall | + holdsBodyOnly(package, receiverName, bodySetterCall, this) and + holdsContentTypeNode(package, receiverName, contentTypeSetterCall, contentTypeNode) + | + contentTypeSetterCall.getReceiver().getAPredecessor*() = + bodySetterCall.getReceiver().getAPredecessor*() + ) + ) + } + + override DataFlow::Node getAContentTypeNode() { result = contentTypeNode.getAPredecessor*() } + + override HTTP::ResponseWriter getResponseWriter() { none() } + } + + // Holds for a call that sets the body. + private predicate holdsBodyOnly( + string package, string receiverName, DataFlow::CallNode bodySetterCall, DataFlow::Node bodyNode + ) { + exists(string methodName, Method m | + m.hasQualifiedName(package, receiverName, methodName) and + bodySetterCall = m.getACall() + | + package = packagePath() and + ( + // Receiver type: Context + receiverName = "Context" and + ( + // signature: func (*Context).Write(data []byte) (int, error) + methodName = "Write" and + bodyNode = bodySetterCall.getArgument(0) + or + // signature: func (*Context).WriteString(data string) (int, error) + methodName = "WriteString" and + bodyNode = bodySetterCall.getArgument(0) + ) + ) + ) + } + + // Holds for a call that sets the body; the content-type is implicit. + private predicate holdsBodyAndContentTypeString( + string package, string receiverName, DataFlow::Node bodyNode, string contentTypeString + ) { + // One call sets both body and content-type (which is implicit in the func name). + exists(string methodName, Method m, DataFlow::CallNode bodySetterCall | + m.hasQualifiedName(package, receiverName, methodName) and + bodySetterCall = m.getACall() + | + package = packagePath() and + ( + // Receiver type: Context + receiverName = "Context" and + ( + // signature: func (*Context).Error(code int, msg string) error + methodName = "Error" and + bodyNode = bodySetterCall.getArgument(1) and + contentTypeString = "text/plain" + or + // signature: func (*Context).HTML(code int, html string) error + methodName = "HTML" and + bodyNode = bodySetterCall.getArgument(1) and + contentTypeString = "text/html" + or + // signature: func (*Context).HTMLBlob(code int, bs []byte) error + methodName = "HTMLBlob" and + bodyNode = bodySetterCall.getArgument(1) and + contentTypeString = "text/html" + or + // signature: func (*Context).JSON(code int, data interface{}) error + methodName = "JSON" and + bodyNode = bodySetterCall.getArgument(1) and + contentTypeString = "application/json" + or + // signature: func (*Context).JSONBlob(code int, bs []byte) error + methodName = "JSONBlob" and + bodyNode = bodySetterCall.getArgument(1) and + contentTypeString = "application/json" + or + // signature: func (*Context).JSONP(code int, data interface{}) error + methodName = "JSONP" and + bodyNode = bodySetterCall.getArgument(1) and + contentTypeString = "application/javascript" + or + // signature: func (*Context).JSONPBlob(code int, bs []byte) error + methodName = "JSONPBlob" and + bodyNode = bodySetterCall.getArgument(1) and + contentTypeString = "application/javascript" + or + // signature: func (*Context).JSONPCallback(code int, callback string, data interface{}) error + methodName = "JSONPCallback" and + bodyNode = bodySetterCall.getArgument(2) and + contentTypeString = "application/javascript" + or + // signature: func (*Context).JSONPCallbackBlob(code int, callback string, bs []byte) (err error) + methodName = "JSONPCallbackBlob" and + bodyNode = bodySetterCall.getArgument(2) and + contentTypeString = "application/javascript" + or + // signature: func (*Context).String(code int, s string) error + methodName = "String" and + bodyNode = bodySetterCall.getArgument(1) and + contentTypeString = "text/plain" + or + // signature: func (*Context).StringBlob(code int, bs []byte) error + methodName = "StringBlob" and + bodyNode = bodySetterCall.getArgument(1) and + contentTypeString = "text/plain" + or + // signature: func (*Context).Stringf(code int, format string, a ...interface{}) error + methodName = "Stringf" and + bodyNode = bodySetterCall.getArgument([1, any(int i | i >= 2)]) and + contentTypeString = "text/plain" + or + // signature: func (*Context).XML(code int, data interface{}) error + methodName = "XML" and + bodyNode = bodySetterCall.getArgument(1) and + contentTypeString = "text/xml" + or + // signature: func (*Context).XMLBlob(code int, bs []byte) error + methodName = "XMLBlob" and + bodyNode = bodySetterCall.getArgument(1) and + contentTypeString = "text/xml" + ) + ) + ) + } + + // Holds for a call that sets the body; the content-type is a parameter. + private predicate holdsBodyAndContentTypeNode( + string package, string receiverName, DataFlow::Node bodyNode, DataFlow::Node contentTypeNode + ) { + exists(string methodName, Method m, DataFlow::CallNode bodySetterCall | + m.hasQualifiedName(package, receiverName, methodName) and + bodySetterCall = m.getACall() + | + package = packagePath() and + ( + // Receiver type: Context + receiverName = "Context" and + ( + // signature: func (*Context).Blob(code int, contentType string, bs []byte) (err error) + methodName = "Blob" and + bodyNode = bodySetterCall.getArgument(2) and + contentTypeNode = bodySetterCall.getArgument(1) + or + // signature: func (*Context).Emit(code int, contentType string, body string) (err error) + methodName = "Emit" and + bodyNode = bodySetterCall.getArgument(2) and + contentTypeNode = bodySetterCall.getArgument(1) + ) + ) + ) + } + + // Holds for a call that sets the content-type (implicit). + private predicate holdsContentTypeString( + string package, string receiverName, DataFlow::CallNode contentTypeSetterCall, + string contentType + ) { + exists(string methodName, Method m | + m.hasQualifiedName(package, receiverName, methodName) and + contentTypeSetterCall = m.getACall() + | + package = packagePath() and + ( + // Receiver type: Context + receiverName = "Context" and + ( + // signature: func (*Context).SetContentTypeHTML() + methodName = "SetContentTypeHTML" and + contentType = "text/html" + or + // signature: func (*Context).SetContentTypeJSON() + methodName = "SetContentTypeJSON" and + contentType = "application/json" + or + // signature: func (*Context).SetContentTypeText() + methodName = "SetContentTypeText" and + contentType = "text/plain" + or + // signature: func (*Context).SetContentTypeXML() + methodName = "SetContentTypeXML" and + contentType = "text/xml" + ) + ) + ) + } + + // Holds for a call that sets the content-type via a parameter. + private predicate holdsContentTypeNode( + string package, string receiverName, DataFlow::CallNode contentTypeSetterCall, + DataFlow::Node contentTypeNode + ) { + exists(string methodName, Method m | + m.hasQualifiedName(package, receiverName, methodName) and + contentTypeSetterCall = m.getACall() + | + package = packagePath() and + ( + // Receiver type: Context + receiverName = "Context" and + ( + // signature: func (*Context).SetContentType(v string) + methodName = "SetContentType" and + contentTypeNode = contentTypeSetterCall.getArgument(0) + ) + ) + ) + } + /** * Models a HTTP header writer model for package: clevergo.tech/clevergo@v0.5.2 */ diff --git a/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.go b/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.go index d3c1b6ea3c0..94d8f8f50cc 100644 --- a/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.go +++ b/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.go @@ -105,7 +105,8 @@ func HttpResponseBody_ClevergoTechClevergov052() { { bodyByte839 := source().([]byte) var rece clevergo.Context - rece.Blob(0, "application/json", bodyByte839) // $contentType=application/json $responseBody=bodyByte839 + ct := "application/json" + rece.Blob(0, ct, bodyByte839) // $contentType=application/json $responseBody=bodyByte839 } // func (*Context).Emit(code int, contentType string, body string) (err error) { From 989bfa2b1dadf6adaee611ad7730bed5e48531a6 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Wed, 14 Apr 2021 19:26:49 +0200 Subject: [PATCH 03/10] Improve naming and comments. --- ql/src/experimental/frameworks/CleverGo.qll | 36 ++++++++++----------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/ql/src/experimental/frameworks/CleverGo.qll b/ql/src/experimental/frameworks/CleverGo.qll index fc9e3e08bdb..cb4c3ff1a7c 100644 --- a/ql/src/experimental/frameworks/CleverGo.qll +++ b/ql/src/experimental/frameworks/CleverGo.qll @@ -196,18 +196,18 @@ private module CleverGo { } /** - * Models HTTP ResponseBody. + * Models HTTP ResponseBody where the content-type is static and non-modifiable. */ - private class HttpResponseBodyContentTypeString extends HTTP::ResponseBody::Range { + private class HttpResponseBodyStaticContentType extends HTTP::ResponseBody::Range { string contentTypeString; - HttpResponseBodyContentTypeString() { + HttpResponseBodyStaticContentType() { exists(string package, string receiverName | - holdsBodyAndContentTypeString(package, receiverName, this, contentTypeString) + setsBodyAndStaticContentType(package, receiverName, this, contentTypeString) or exists(DataFlow::CallNode bodySetterCall, DataFlow::CallNode contentTypeSetterCall | - holdsBodyOnly(package, receiverName, bodySetterCall, this) and - holdsContentTypeString(package, receiverName, contentTypeSetterCall, contentTypeString) + setsBody(package, receiverName, bodySetterCall, this) and + setsStaticContentType(package, receiverName, contentTypeSetterCall, contentTypeString) | contentTypeSetterCall.getReceiver().getAPredecessor*() = bodySetterCall.getReceiver().getAPredecessor*() @@ -221,18 +221,18 @@ private module CleverGo { } /** - * Models HTTP ResponseBody. + * Models HTTP ResponseBody where the content-type can be dynamically set by the caller. */ - private class HttpResponseBodyContentTypeNode extends HTTP::ResponseBody::Range { + private class HttpResponseBodyDynamicContentType extends HTTP::ResponseBody::Range { DataFlow::Node contentTypeNode; - HttpResponseBodyContentTypeNode() { + HttpResponseBodyDynamicContentType() { exists(string package, string receiverName | - holdsBodyAndContentTypeNode(package, receiverName, this, contentTypeNode) + setsBodyAndDynamicContentType(package, receiverName, this, contentTypeNode) or exists(DataFlow::CallNode bodySetterCall, DataFlow::CallNode contentTypeSetterCall | - holdsBodyOnly(package, receiverName, bodySetterCall, this) and - holdsContentTypeNode(package, receiverName, contentTypeSetterCall, contentTypeNode) + setsBody(package, receiverName, bodySetterCall, this) and + setsDynamicContentType(package, receiverName, contentTypeSetterCall, contentTypeNode) | contentTypeSetterCall.getReceiver().getAPredecessor*() = bodySetterCall.getReceiver().getAPredecessor*() @@ -246,7 +246,7 @@ private module CleverGo { } // Holds for a call that sets the body. - private predicate holdsBodyOnly( + private predicate setsBody( string package, string receiverName, DataFlow::CallNode bodySetterCall, DataFlow::Node bodyNode ) { exists(string methodName, Method m | @@ -270,8 +270,8 @@ private module CleverGo { ) } - // Holds for a call that sets the body; the content-type is implicit. - private predicate holdsBodyAndContentTypeString( + // Holds for a call that sets the body; the content-type is static and implicit. + private predicate setsBodyAndStaticContentType( string package, string receiverName, DataFlow::Node bodyNode, string contentTypeString ) { // One call sets both body and content-type (which is implicit in the func name). @@ -359,7 +359,7 @@ private module CleverGo { } // Holds for a call that sets the body; the content-type is a parameter. - private predicate holdsBodyAndContentTypeNode( + private predicate setsBodyAndDynamicContentType( string package, string receiverName, DataFlow::Node bodyNode, DataFlow::Node contentTypeNode ) { exists(string methodName, Method m, DataFlow::CallNode bodySetterCall | @@ -386,7 +386,7 @@ private module CleverGo { } // Holds for a call that sets the content-type (implicit). - private predicate holdsContentTypeString( + private predicate setsStaticContentType( string package, string receiverName, DataFlow::CallNode contentTypeSetterCall, string contentType ) { @@ -420,7 +420,7 @@ private module CleverGo { } // Holds for a call that sets the content-type via a parameter. - private predicate holdsContentTypeNode( + private predicate setsDynamicContentType( string package, string receiverName, DataFlow::CallNode contentTypeSetterCall, DataFlow::Node contentTypeNode ) { From 36396df27187b2aa791d3e26343bef10782576d6 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Wed, 14 Apr 2021 19:33:35 +0200 Subject: [PATCH 04/10] HttpResponseBody: Move .getAPredecessor*() to the test query. --- ql/src/experimental/frameworks/CleverGo.qll | 2 +- .../experimental/frameworks/CleverGo/HttpResponseBody.ql | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ql/src/experimental/frameworks/CleverGo.qll b/ql/src/experimental/frameworks/CleverGo.qll index cb4c3ff1a7c..1ff9ac4dacc 100644 --- a/ql/src/experimental/frameworks/CleverGo.qll +++ b/ql/src/experimental/frameworks/CleverGo.qll @@ -240,7 +240,7 @@ private module CleverGo { ) } - override DataFlow::Node getAContentTypeNode() { result = contentTypeNode.getAPredecessor*() } + override DataFlow::Node getAContentTypeNode() { result = contentTypeNode } override HTTP::ResponseWriter getResponseWriter() { none() } } diff --git a/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.ql b/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.ql index 34a21ef49f6..0e6493e869f 100644 --- a/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.ql +++ b/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.ql @@ -11,8 +11,13 @@ class HttpResponseBodyTest extends InlineExpectationsTest { exists(HTTP::ResponseBody rd | rd.hasLocationInfo(file, line, _, _, _) and ( - element = rd.getAContentType().toString() and - value = rd.getAContentType().toString() and + ( + element = rd.getAContentType().toString() and + value = rd.getAContentType() + or + element = rd.getAContentTypeNode().toString() and + value = rd.getAContentTypeNode().getAPredecessor*().getStringValue() + ) and tag = "contentType" or element = rd.toString() and From 3fd2c7d4bbfc3c4a98d8ce628244e92909eecac6 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Fri, 16 Apr 2021 15:17:23 +0100 Subject: [PATCH 05/10] Note response writers for existing HeaderWrite and HttpRedirect instances --- ql/src/experimental/frameworks/CleverGo.qll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ql/src/experimental/frameworks/CleverGo.qll b/ql/src/experimental/frameworks/CleverGo.qll index 1ff9ac4dacc..575b0f9ef15 100644 --- a/ql/src/experimental/frameworks/CleverGo.qll +++ b/ql/src/experimental/frameworks/CleverGo.qll @@ -192,7 +192,7 @@ private module CleverGo { override DataFlow::Node getUrl() { result = urlNode } - override HTTP::ResponseWriter getResponseWriter() { none() } + override HTTP::ResponseWriter getResponseWriter() { result.getANode() = this.getReceiver() } } /** @@ -455,6 +455,6 @@ private module CleverGo { override DataFlow::Node getValue() { result = this.getArgument(1) } - override HTTP::ResponseWriter getResponseWriter() { none() } + override HTTP::ResponseWriter getResponseWriter() { result.getANode() = this.getReceiver() } } } From 9ea8b34e47a4f78df0dfeaae07572157a2197464 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Fri, 16 Apr 2021 15:19:06 +0100 Subject: [PATCH 06/10] HTTP ResponseBody: support HeaderWrites with hard-coded header values. --- ql/src/semmle/go/concepts/HTTP.qll | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ql/src/semmle/go/concepts/HTTP.qll b/ql/src/semmle/go/concepts/HTTP.qll index 6763b6e29cb..3624060ffbd 100644 --- a/ql/src/semmle/go/concepts/HTTP.qll +++ b/ql/src/semmle/go/concepts/HTTP.qll @@ -190,7 +190,14 @@ module HTTP { abstract ResponseWriter getResponseWriter(); /** Gets a content-type associated with this body. */ - string getAContentType() { result = getAContentTypeNode().getStringValue() } + string getAContentType() { + exists(HTTP::HeaderWrite hw | hw = getResponseWriter().getAHeaderWrite() | + hw.getHeaderName() = "content-type" and + result = hw.getHeaderValue() + ) + or + result = getAContentTypeNode().getStringValue() + } /** Gets a dataflow node for a content-type associated with this body. */ DataFlow::Node getAContentTypeNode() { From 0beaa7fdc95c0b3f20c916224fa8079d09f692c8 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Fri, 16 Apr 2021 15:37:45 +0100 Subject: [PATCH 07/10] Model content-type setters as HeaderWrites. --- ql/src/experimental/frameworks/CleverGo.qll | 108 ++++++++++++++------ 1 file changed, 77 insertions(+), 31 deletions(-) diff --git a/ql/src/experimental/frameworks/CleverGo.qll b/ql/src/experimental/frameworks/CleverGo.qll index 575b0f9ef15..43214ea6d80 100644 --- a/ql/src/experimental/frameworks/CleverGo.qll +++ b/ql/src/experimental/frameworks/CleverGo.qll @@ -200,24 +200,17 @@ private module CleverGo { */ private class HttpResponseBodyStaticContentType extends HTTP::ResponseBody::Range { string contentTypeString; + DataFlow::Node receiverNode; HttpResponseBodyStaticContentType() { exists(string package, string receiverName | - setsBodyAndStaticContentType(package, receiverName, this, contentTypeString) - or - exists(DataFlow::CallNode bodySetterCall, DataFlow::CallNode contentTypeSetterCall | - setsBody(package, receiverName, bodySetterCall, this) and - setsStaticContentType(package, receiverName, contentTypeSetterCall, contentTypeString) - | - contentTypeSetterCall.getReceiver().getAPredecessor*() = - bodySetterCall.getReceiver().getAPredecessor*() - ) + setsBodyAndStaticContentType(package, receiverName, this, contentTypeString, receiverNode) ) } override string getAContentType() { result = contentTypeString } - override HTTP::ResponseWriter getResponseWriter() { none() } + override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } } /** @@ -225,33 +218,80 @@ private module CleverGo { */ private class HttpResponseBodyDynamicContentType extends HTTP::ResponseBody::Range { DataFlow::Node contentTypeNode; + DataFlow::Node receiverNode; HttpResponseBodyDynamicContentType() { exists(string package, string receiverName | - setsBodyAndDynamicContentType(package, receiverName, this, contentTypeNode) - or - exists(DataFlow::CallNode bodySetterCall, DataFlow::CallNode contentTypeSetterCall | - setsBody(package, receiverName, bodySetterCall, this) and - setsDynamicContentType(package, receiverName, contentTypeSetterCall, contentTypeNode) - | - contentTypeSetterCall.getReceiver().getAPredecessor*() = - bodySetterCall.getReceiver().getAPredecessor*() - ) + setsBodyAndDynamicContentType(package, receiverName, this, contentTypeNode, receiverNode) ) } override DataFlow::Node getAContentTypeNode() { result = contentTypeNode } - override HTTP::ResponseWriter getResponseWriter() { none() } + override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } + } + + /** + * Models HTTP ResponseBody where the content-type is set by another call. + */ + private class HttpResponseBodyNoContentType extends HTTP::ResponseBody::Range { + private DataFlow::Node receiverNode; + + HttpResponseBodyNoContentType() { + exists(string package, string receiverName | + setsBody(package, receiverName, receiverNode, this) + ) + } + + override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } + } + + /** + * Models an HTTP static content-type setter for package: clevergo.tech/clevergo@v0.5.2 + */ + private class StaticContentTypeSetter extends HTTP::HeaderWrite::Range, DataFlow::CallNode { + DataFlow::Node receiverNode; + string contentType; + + StaticContentTypeSetter() { setsStaticContentType(_, _, this, contentType, receiverNode) } + + override string getHeaderName() { result = "content-type" } + + override string getHeaderValue() { result = contentType } + + override DataFlow::Node getName() { none() } + + override DataFlow::Node getValue() { none() } + + override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } + } + + /** + * Models an HTTP dynamic content-type setter for package: clevergo.tech/clevergo@v0.5.2 + */ + private class DynamicContentTypeSetter extends HTTP::HeaderWrite::Range, DataFlow::CallNode { + DataFlow::Node receiverNode; + DataFlow::Node contentType; + + DynamicContentTypeSetter() { setsDynamicContentType(_, _, this, contentType, receiverNode) } + + override string getHeaderName() { result = "content-type" } + + override DataFlow::Node getName() { none() } + + override DataFlow::Node getValue() { result = contentType } + + override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } } // Holds for a call that sets the body. private predicate setsBody( - string package, string receiverName, DataFlow::CallNode bodySetterCall, DataFlow::Node bodyNode + string package, string receiverName, DataFlow::Node receiverNode, DataFlow::Node bodyNode ) { - exists(string methodName, Method m | + exists(string methodName, Method m, DataFlow::CallNode bodySetterCall | m.hasQualifiedName(package, receiverName, methodName) and - bodySetterCall = m.getACall() + bodySetterCall = m.getACall() and + receiverNode = bodySetterCall.getReceiver() | package = packagePath() and ( @@ -272,12 +312,14 @@ private module CleverGo { // Holds for a call that sets the body; the content-type is static and implicit. private predicate setsBodyAndStaticContentType( - string package, string receiverName, DataFlow::Node bodyNode, string contentTypeString + string package, string receiverName, DataFlow::Node bodyNode, string contentTypeString, + DataFlow::Node receiverNode ) { // One call sets both body and content-type (which is implicit in the func name). exists(string methodName, Method m, DataFlow::CallNode bodySetterCall | m.hasQualifiedName(package, receiverName, methodName) and - bodySetterCall = m.getACall() + bodySetterCall = m.getACall() and + receiverNode = bodySetterCall.getReceiver() | package = packagePath() and ( @@ -360,11 +402,13 @@ private module CleverGo { // Holds for a call that sets the body; the content-type is a parameter. private predicate setsBodyAndDynamicContentType( - string package, string receiverName, DataFlow::Node bodyNode, DataFlow::Node contentTypeNode + string package, string receiverName, DataFlow::Node bodyNode, DataFlow::Node contentTypeNode, + DataFlow::Node receiverNode ) { exists(string methodName, Method m, DataFlow::CallNode bodySetterCall | m.hasQualifiedName(package, receiverName, methodName) and - bodySetterCall = m.getACall() + bodySetterCall = m.getACall() and + receiverNode = bodySetterCall.getReceiver() | package = packagePath() and ( @@ -388,11 +432,12 @@ private module CleverGo { // Holds for a call that sets the content-type (implicit). private predicate setsStaticContentType( string package, string receiverName, DataFlow::CallNode contentTypeSetterCall, - string contentType + string contentType, DataFlow::Node receiverNode ) { exists(string methodName, Method m | m.hasQualifiedName(package, receiverName, methodName) and - contentTypeSetterCall = m.getACall() + contentTypeSetterCall = m.getACall() and + receiverNode = contentTypeSetterCall.getReceiver() | package = packagePath() and ( @@ -422,11 +467,12 @@ private module CleverGo { // Holds for a call that sets the content-type via a parameter. private predicate setsDynamicContentType( string package, string receiverName, DataFlow::CallNode contentTypeSetterCall, - DataFlow::Node contentTypeNode + DataFlow::Node contentTypeNode, DataFlow::Node receiverNode ) { exists(string methodName, Method m | m.hasQualifiedName(package, receiverName, methodName) and - contentTypeSetterCall = m.getACall() + contentTypeSetterCall = m.getACall() and + receiverNode = contentTypeSetterCall.getReceiver() | package = packagePath() and ( From 5578afa189cacdd0ad4d7770c74be1d9d493c693 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Wed, 28 Apr 2021 17:18:52 +0200 Subject: [PATCH 08/10] Regenerate using latest codemill generator. --- ql/src/experimental/frameworks/CleverGo.qll | 287 ++++++++++-------- .../frameworks/CleverGo/HeaderWrite.go | 46 ++- .../frameworks/CleverGo/HeaderWrite.ql | 35 ++- .../frameworks/CleverGo/HttpRedirect.go | 2 +- .../frameworks/CleverGo/HttpRedirect.ql | 2 +- .../frameworks/CleverGo/HttpResponseBody.go | 65 +--- .../frameworks/CleverGo/HttpResponseBody.ql | 11 +- .../frameworks/CleverGo/TaintTracking.go | 2 +- .../frameworks/CleverGo/TaintTracking.ql | 2 +- .../frameworks/CleverGo/UntrustedSources.go | 2 +- .../frameworks/CleverGo/UntrustedSources.ql | 2 +- 11 files changed, 243 insertions(+), 213 deletions(-) diff --git a/ql/src/experimental/frameworks/CleverGo.qll b/ql/src/experimental/frameworks/CleverGo.qll index 43214ea6d80..5d9b2e19855 100644 --- a/ql/src/experimental/frameworks/CleverGo.qll +++ b/ql/src/experimental/frameworks/CleverGo.qll @@ -8,8 +8,6 @@ import go * Provides classes for working with concepts from the [`clevergo.tech/clevergo@v0.5.2`](https://pkg.go.dev/clevergo.tech/clevergo@v0.5.2) package. */ private module CleverGo { - /** Gets the package path. */ - bindingset[result] string packagePath() { result = package(["clevergo.tech/clevergo", "github.com/clevergo/clevergo"], "") } @@ -213,112 +211,14 @@ private module CleverGo { override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } } - /** - * Models HTTP ResponseBody where the content-type can be dynamically set by the caller. - */ - private class HttpResponseBodyDynamicContentType extends HTTP::ResponseBody::Range { - DataFlow::Node contentTypeNode; - DataFlow::Node receiverNode; - - HttpResponseBodyDynamicContentType() { - exists(string package, string receiverName | - setsBodyAndDynamicContentType(package, receiverName, this, contentTypeNode, receiverNode) - ) - } - - override DataFlow::Node getAContentTypeNode() { result = contentTypeNode } - - override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } - } - - /** - * Models HTTP ResponseBody where the content-type is set by another call. - */ - private class HttpResponseBodyNoContentType extends HTTP::ResponseBody::Range { - private DataFlow::Node receiverNode; - - HttpResponseBodyNoContentType() { - exists(string package, string receiverName | - setsBody(package, receiverName, receiverNode, this) - ) - } - - override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } - } - - /** - * Models an HTTP static content-type setter for package: clevergo.tech/clevergo@v0.5.2 - */ - private class StaticContentTypeSetter extends HTTP::HeaderWrite::Range, DataFlow::CallNode { - DataFlow::Node receiverNode; - string contentType; - - StaticContentTypeSetter() { setsStaticContentType(_, _, this, contentType, receiverNode) } - - override string getHeaderName() { result = "content-type" } - - override string getHeaderValue() { result = contentType } - - override DataFlow::Node getName() { none() } - - override DataFlow::Node getValue() { none() } - - override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } - } - - /** - * Models an HTTP dynamic content-type setter for package: clevergo.tech/clevergo@v0.5.2 - */ - private class DynamicContentTypeSetter extends HTTP::HeaderWrite::Range, DataFlow::CallNode { - DataFlow::Node receiverNode; - DataFlow::Node contentType; - - DynamicContentTypeSetter() { setsDynamicContentType(_, _, this, contentType, receiverNode) } - - override string getHeaderName() { result = "content-type" } - - override DataFlow::Node getName() { none() } - - override DataFlow::Node getValue() { result = contentType } - - override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } - } - - // Holds for a call that sets the body. - private predicate setsBody( - string package, string receiverName, DataFlow::Node receiverNode, DataFlow::Node bodyNode - ) { - exists(string methodName, Method m, DataFlow::CallNode bodySetterCall | - m.hasQualifiedName(package, receiverName, methodName) and - bodySetterCall = m.getACall() and - receiverNode = bodySetterCall.getReceiver() - | - package = packagePath() and - ( - // Receiver type: Context - receiverName = "Context" and - ( - // signature: func (*Context).Write(data []byte) (int, error) - methodName = "Write" and - bodyNode = bodySetterCall.getArgument(0) - or - // signature: func (*Context).WriteString(data string) (int, error) - methodName = "WriteString" and - bodyNode = bodySetterCall.getArgument(0) - ) - ) - ) - } - - // Holds for a call that sets the body; the content-type is static and implicit. + // Holds for a call that sets the body; the content-type is implicitly set. private predicate setsBodyAndStaticContentType( string package, string receiverName, DataFlow::Node bodyNode, string contentTypeString, DataFlow::Node receiverNode ) { - // One call sets both body and content-type (which is implicit in the func name). - exists(string methodName, Method m, DataFlow::CallNode bodySetterCall | - m.hasQualifiedName(package, receiverName, methodName) and - bodySetterCall = m.getACall() and + exists(string methodName, Method met, DataFlow::CallNode bodySetterCall | + met.hasQualifiedName(package, receiverName, methodName) and + bodySetterCall = met.getACall() and receiverNode = bodySetterCall.getReceiver() | package = packagePath() and @@ -400,14 +300,33 @@ private module CleverGo { ) } + /** + * Models HTTP ResponseBody where the content-type can be dynamically set by the caller. + */ + private class HttpResponseBodyDynamicContentType extends HTTP::ResponseBody::Range { + DataFlow::Node contentTypeNode; + DataFlow::Node receiverNode; + + HttpResponseBodyDynamicContentType() { + exists(string package, string receiverName | + setsBodyAndDynamicContentType(package, receiverName, this, contentTypeNode, receiverNode) + ) + } + + override DataFlow::Node getAContentTypeNode() { result = contentTypeNode } + + override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } + } + // Holds for a call that sets the body; the content-type is a parameter. + // Both body and content-type are parameters in the same func call. private predicate setsBodyAndDynamicContentType( string package, string receiverName, DataFlow::Node bodyNode, DataFlow::Node contentTypeNode, DataFlow::Node receiverNode ) { - exists(string methodName, Method m, DataFlow::CallNode bodySetterCall | - m.hasQualifiedName(package, receiverName, methodName) and - bodySetterCall = m.getACall() and + exists(string methodName, Method met, DataFlow::CallNode bodySetterCall | + met.hasQualifiedName(package, receiverName, methodName) and + bodySetterCall = met.getACall() and receiverNode = bodySetterCall.getReceiver() | package = packagePath() and @@ -429,14 +348,109 @@ private module CleverGo { ) } + /** + * Models HTTP ResponseBody where only the body is set. + */ + private class HttpResponseBodyNoContentType extends HTTP::ResponseBody::Range { + DataFlow::Node receiverNode; + + HttpResponseBodyNoContentType() { + exists(string package, string receiverName | + setsBody(package, receiverName, receiverNode, this) + ) + } + + override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } + } + + // Holds for a call that sets the body. The content-type is not defined. + private predicate setsBody( + string package, string receiverName, DataFlow::Node receiverNode, DataFlow::Node bodyNode + ) { + exists(string methodName, Method met, DataFlow::CallNode bodySetterCall | + met.hasQualifiedName(package, receiverName, methodName) and + bodySetterCall = met.getACall() and + receiverNode = bodySetterCall.getReceiver() + | + package = packagePath() and + ( + // Receiver type: Context + receiverName = "Context" and + ( + // signature: func (*Context).Write(data []byte) (int, error) + methodName = "Write" and + bodyNode = bodySetterCall.getArgument(0) + or + // signature: func (*Context).WriteString(data string) (int, error) + methodName = "WriteString" and + bodyNode = bodySetterCall.getArgument(0) + ) + ) + ) + } + + /** + * Models HTTP header writer models for package: clevergo.tech/clevergo@v0.5.2 + */ + private class HeaderWrite extends HTTP::HeaderWrite::Range, DataFlow::CallNode { + string receiverName; + string methodName; + DataFlow::Node headerNameNode; + DataFlow::Node headerValueNode; + + HeaderWrite() { + ( + // Type methods: + this = + any(Method m | m.hasQualifiedName(packagePath(), receiverName, methodName)).getACall() and + ( + // Receiver type: Context + receiverName = "Context" and + ( + // signature: func (*Context).SetHeader(key string, value string) + methodName = "SetHeader" and + headerNameNode = this.getArgument(0) and + headerValueNode = this.getArgument(1) + ) + ) + ) + } + + override DataFlow::Node getName() { result = headerNameNode } + + override DataFlow::Node getValue() { result = headerValueNode } + + override HTTP::ResponseWriter getResponseWriter() { none() } + } + + /** + * Models an HTTP static content-type setter. + */ + private class StaticContentTypeSetter extends HTTP::HeaderWrite::Range, DataFlow::CallNode { + DataFlow::Node receiverNode; + string contentTypeString; + + StaticContentTypeSetter() { setsStaticContentType(_, _, this, contentTypeString, receiverNode) } + + override string getHeaderName() { result = "content-type" } + + override string getHeaderValue() { result = contentTypeString } + + override DataFlow::Node getName() { none() } + + override DataFlow::Node getValue() { none() } + + override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } + } + // Holds for a call that sets the content-type (implicit). private predicate setsStaticContentType( string package, string receiverName, DataFlow::CallNode contentTypeSetterCall, - string contentType, DataFlow::Node receiverNode + string contentTypeString, DataFlow::Node receiverNode ) { - exists(string methodName, Method m | - m.hasQualifiedName(package, receiverName, methodName) and - contentTypeSetterCall = m.getACall() and + exists(string methodName, Method met | + met.hasQualifiedName(package, receiverName, methodName) and + contentTypeSetterCall = met.getACall() and receiverNode = contentTypeSetterCall.getReceiver() | package = packagePath() and @@ -446,32 +460,52 @@ private module CleverGo { ( // signature: func (*Context).SetContentTypeHTML() methodName = "SetContentTypeHTML" and - contentType = "text/html" + contentTypeString = "text/html" or // signature: func (*Context).SetContentTypeJSON() methodName = "SetContentTypeJSON" and - contentType = "application/json" + contentTypeString = "application/json" or // signature: func (*Context).SetContentTypeText() methodName = "SetContentTypeText" and - contentType = "text/plain" + contentTypeString = "text/plain" or // signature: func (*Context).SetContentTypeXML() methodName = "SetContentTypeXML" and - contentType = "text/xml" + contentTypeString = "text/xml" ) ) ) } + /** + * Models an HTTP dynamic content-type setter. + */ + private class DynamicContentTypeSetter extends HTTP::HeaderWrite::Range, DataFlow::CallNode { + DataFlow::Node receiverNode; + DataFlow::Node contentTypeNode; + + DynamicContentTypeSetter() { setsDynamicContentType(_, _, this, contentTypeNode, receiverNode) } + + override string getHeaderName() { result = "content-type" } + + override string getHeaderValue() { none() } + + override DataFlow::Node getName() { none() } + + override DataFlow::Node getValue() { result = contentTypeNode } + + override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } + } + // Holds for a call that sets the content-type via a parameter. private predicate setsDynamicContentType( string package, string receiverName, DataFlow::CallNode contentTypeSetterCall, DataFlow::Node contentTypeNode, DataFlow::Node receiverNode ) { - exists(string methodName, Method m | - m.hasQualifiedName(package, receiverName, methodName) and - contentTypeSetterCall = m.getACall() and + exists(string methodName, Method met | + met.hasQualifiedName(package, receiverName, methodName) and + contentTypeSetterCall = met.getACall() and receiverNode = contentTypeSetterCall.getReceiver() | package = packagePath() and @@ -486,21 +520,4 @@ private module CleverGo { ) ) } - - /** - * Models a HTTP header writer model for package: clevergo.tech/clevergo@v0.5.2 - */ - private class HeaderWrite extends HTTP::HeaderWrite::Range, DataFlow::CallNode { - HeaderWrite() { - // Receiver type: Context - // signature: func (*Context).SetHeader(key string, value string) - this = any(Method m | m.hasQualifiedName(packagePath(), "Context", "SetHeader")).getACall() - } - - override DataFlow::Node getName() { result = this.getArgument(0) } - - override DataFlow::Node getValue() { result = this.getArgument(1) } - - override HTTP::ResponseWriter getResponseWriter() { result.getANode() = this.getReceiver() } - } } diff --git a/ql/test/experimental/frameworks/CleverGo/HeaderWrite.go b/ql/test/experimental/frameworks/CleverGo/HeaderWrite.go index 50ae952e98b..f14e30e8038 100644 --- a/ql/test/experimental/frameworks/CleverGo/HeaderWrite.go +++ b/ql/test/experimental/frameworks/CleverGo/HeaderWrite.go @@ -5,17 +5,55 @@ package main import "clevergo.tech/clevergo" // Package clevergo.tech/clevergo@v0.5.2 -func HeaderWrite_ClevergoTechClevergov052() { +func HeaderWrite_ClevergoTechClevergoV052() { // Header write via method calls. { // Header write via method calls on clevergo.tech/clevergo.Context. { // func (*Context).SetHeader(key string, value string) { - keyString566 := source().(string) - valString497 := source().(string) + keyString506 := source().(string) + valString213 := source().(string) var rece clevergo.Context - rece.SetHeader(keyString566, valString497) // $headerKey=keyString566 $headerVal=valString497 + rece.SetHeader(keyString506, valString213) // $headerKeyNode=keyString506 $headerValNode=valString213 + } + } + } + // Dynamic Content-Type header via method calls. + { + // Dynamic Content-Type header via method calls on clevergo.tech/clevergo.Context. + { + // func (*Context).SetContentType(v string) + { + valString468 := source().(string) + var rece clevergo.Context + rece.SetContentType(valString468) // $headerKey=content-type $headerValNode=valString468 + } + } + } + // Static Content-Type header write via method calls. + { + // Static Content-Type header write via method calls on clevergo.tech/clevergo.Context. + { + // func (*Context).SetContentTypeHTML() + { + var rece clevergo.Context + rece.SetContentTypeHTML() // $headerKey=content-type $headerVal=text/html + } + // func (*Context).SetContentTypeJSON() + { + var rece clevergo.Context + rece.SetContentTypeJSON() // $headerKey=content-type $headerVal=application/json + } + // func (*Context).SetContentTypeText() + { + var rece clevergo.Context + rece.SetContentTypeText() // $headerKey=content-type $headerVal=text/plain + } + // func (*Context).SetContentTypeXML() + { + var rece clevergo.Context + rece.SetContentTypeXML() // $headerKey=content-type $headerVal=text/xml } } } diff --git a/ql/test/experimental/frameworks/CleverGo/HeaderWrite.ql b/ql/test/experimental/frameworks/CleverGo/HeaderWrite.ql index 3c8e6d53df2..d2cc16f92cf 100644 --- a/ql/test/experimental/frameworks/CleverGo/HeaderWrite.ql +++ b/ql/test/experimental/frameworks/CleverGo/HeaderWrite.ql @@ -1,22 +1,53 @@ import go -import experimental.frameworks.CleverGo import TestUtilities.InlineExpectationsTest +import experimental.frameworks.CleverGo class HttpHeaderWriteTest extends InlineExpectationsTest { HttpHeaderWriteTest() { this = "HttpHeaderWriteTest" } - override string getARelevantTag() { result = ["headerKey", "headerVal"] } + override string getARelevantTag() { + result = ["headerKeyNode", "headerValNode", "headerKey", "headerVal"] + } override predicate hasActualResult(string file, int line, string element, string tag, string value) { + // Dynamic key-value header: exists(HTTP::HeaderWrite hw | hw.hasLocationInfo(file, line, _, _, _) and ( element = hw.getName().toString() and value = hw.getName().toString() and + tag = "headerKeyNode" + or + element = hw.getValue().toString() and + value = hw.getValue().toString() and + tag = "headerValNode" + ) + ) + or + // Static key, dynamic value header: + exists(HTTP::HeaderWrite hw | + hw.hasLocationInfo(file, line, _, _, _) and + ( + element = hw.getHeaderName().toString() and + value = hw.getHeaderName() and tag = "headerKey" or element = hw.getValue().toString() and value = hw.getValue().toString() and + tag = "headerValNode" + ) + ) + or + // Static key, static value header: + exists(HTTP::HeaderWrite hw | + hw.hasLocationInfo(file, line, _, _, _) and + ( + element = hw.getHeaderName().toString() and + value = hw.getHeaderName() and + tag = "headerKey" + or + element = hw.getHeaderValue().toString() and + value = hw.getHeaderValue() and tag = "headerVal" ) ) diff --git a/ql/test/experimental/frameworks/CleverGo/HttpRedirect.go b/ql/test/experimental/frameworks/CleverGo/HttpRedirect.go index 9559cfef2af..f9ab6e443e1 100644 --- a/ql/test/experimental/frameworks/CleverGo/HttpRedirect.go +++ b/ql/test/experimental/frameworks/CleverGo/HttpRedirect.go @@ -5,7 +5,7 @@ package main import "clevergo.tech/clevergo" // Package clevergo.tech/clevergo@v0.5.2 -func HttpRedirect_ClevergoTechClevergov052() { +func HttpRedirect_ClevergoTechClevergoV052() { // Redirect via method calls. { // Redirect via method calls on clevergo.tech/clevergo.Context. diff --git a/ql/test/experimental/frameworks/CleverGo/HttpRedirect.ql b/ql/test/experimental/frameworks/CleverGo/HttpRedirect.ql index efc79c9d7e8..9ba91ffcf9b 100644 --- a/ql/test/experimental/frameworks/CleverGo/HttpRedirect.ql +++ b/ql/test/experimental/frameworks/CleverGo/HttpRedirect.ql @@ -1,6 +1,6 @@ import go -import experimental.frameworks.CleverGo import TestUtilities.InlineExpectationsTest +import experimental.frameworks.CleverGo class HttpRedirectTest extends InlineExpectationsTest { HttpRedirectTest() { this = "HttpRedirectTest" } diff --git a/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.go b/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.go index 94d8f8f50cc..c27b1fc0097 100644 --- a/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.go +++ b/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.go @@ -5,7 +5,7 @@ package main import "clevergo.tech/clevergo" // Package clevergo.tech/clevergo@v0.5.2 -func HttpResponseBody_ClevergoTechClevergov052() { +func HttpResponseBody_ClevergoTechClevergoV052() { // Response body is set via a method call (the content-type is implicit in the method name). { // Response body is set via a method call on the clevergo.tech/clevergo.Context type (the content-type is implicit in the method name). @@ -105,8 +105,7 @@ func HttpResponseBody_ClevergoTechClevergov052() { { bodyByte839 := source().([]byte) var rece clevergo.Context - ct := "application/json" - rece.Blob(0, ct, bodyByte839) // $contentType=application/json $responseBody=bodyByte839 + rece.Blob(0, "application/json", bodyByte839) // $contentType=application/json $responseBody=bodyByte839 } // func (*Context).Emit(code int, contentType string, body string) (err error) { @@ -116,71 +115,21 @@ func HttpResponseBody_ClevergoTechClevergov052() { } } } - // Response body and content-type are set via calls of different methods. + // Response body is set via a call of a type method. { - // Response body and content-type are set via calls of different methods on the clevergo.tech/clevergo.Context type. + // Response body is set via a call of a method on the clevergo.tech/clevergo.Context type. { // func (*Context).Write(data []byte) (int, error) { bodyByte982 := source().([]byte) var rece clevergo.Context - rece.SetContentType("application/json") - rece.Write(bodyByte982) // $contentType=application/json $responseBody=bodyByte982 - } - { - bodyByte458 := source().([]byte) - var rece clevergo.Context - rece.SetContentTypeHTML() - rece.Write(bodyByte458) // $contentType=text/html $responseBody=bodyByte458 - } - { - bodyByte506 := source().([]byte) - var rece clevergo.Context - rece.SetContentTypeJSON() - rece.Write(bodyByte506) // $contentType=application/json $responseBody=bodyByte506 - } - { - bodyByte213 := source().([]byte) - var rece clevergo.Context - rece.SetContentTypeText() - rece.Write(bodyByte213) // $contentType=text/plain $responseBody=bodyByte213 - } - { - bodyByte468 := source().([]byte) - var rece clevergo.Context - rece.SetContentTypeXML() - rece.Write(bodyByte468) // $contentType=text/xml $responseBody=bodyByte468 + rece.Write(bodyByte982) // $responseBody=bodyByte982 } // func (*Context).WriteString(data string) (int, error) { - bodyString219 := source().(string) + bodyString458 := source().(string) var rece clevergo.Context - rece.SetContentType("application/json") - rece.WriteString(bodyString219) // $contentType=application/json $responseBody=bodyString219 - } - { - bodyString265 := source().(string) - var rece clevergo.Context - rece.SetContentTypeHTML() - rece.WriteString(bodyString265) // $contentType=text/html $responseBody=bodyString265 - } - { - bodyString971 := source().(string) - var rece clevergo.Context - rece.SetContentTypeJSON() - rece.WriteString(bodyString971) // $contentType=application/json $responseBody=bodyString971 - } - { - bodyString320 := source().(string) - var rece clevergo.Context - rece.SetContentTypeText() - rece.WriteString(bodyString320) // $contentType=text/plain $responseBody=bodyString320 - } - { - bodyString545 := source().(string) - var rece clevergo.Context - rece.SetContentTypeXML() - rece.WriteString(bodyString545) // $contentType=text/xml $responseBody=bodyString545 + rece.WriteString(bodyString458) // $responseBody=bodyString458 } } } diff --git a/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.ql b/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.ql index 0e6493e869f..8cfb0dee469 100644 --- a/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.ql +++ b/ql/test/experimental/frameworks/CleverGo/HttpResponseBody.ql @@ -1,6 +1,6 @@ import go -import experimental.frameworks.CleverGo import TestUtilities.InlineExpectationsTest +import experimental.frameworks.CleverGo class HttpResponseBodyTest extends InlineExpectationsTest { HttpResponseBodyTest() { this = "HttpResponseBodyTest" } @@ -11,13 +11,8 @@ class HttpResponseBodyTest extends InlineExpectationsTest { exists(HTTP::ResponseBody rd | rd.hasLocationInfo(file, line, _, _, _) and ( - ( - element = rd.getAContentType().toString() and - value = rd.getAContentType() - or - element = rd.getAContentTypeNode().toString() and - value = rd.getAContentTypeNode().getAPredecessor*().getStringValue() - ) and + element = rd.getAContentType().toString() and + value = rd.getAContentType().toString() and tag = "contentType" or element = rd.toString() and diff --git a/ql/test/experimental/frameworks/CleverGo/TaintTracking.go b/ql/test/experimental/frameworks/CleverGo/TaintTracking.go index 9096439d9fd..614e0fed03d 100644 --- a/ql/test/experimental/frameworks/CleverGo/TaintTracking.go +++ b/ql/test/experimental/frameworks/CleverGo/TaintTracking.go @@ -10,7 +10,7 @@ import ( ) // Package clevergo.tech/clevergo@v0.5.2 -func TaintTracking_ClevergoTechClevergov052() { +func TaintTracking_ClevergoTechClevergoV052() { // Taint-tracking through functions. { // func CleanPath(p string) string diff --git a/ql/test/experimental/frameworks/CleverGo/TaintTracking.ql b/ql/test/experimental/frameworks/CleverGo/TaintTracking.ql index c828bf5380d..bd15905bba9 100644 --- a/ql/test/experimental/frameworks/CleverGo/TaintTracking.ql +++ b/ql/test/experimental/frameworks/CleverGo/TaintTracking.ql @@ -1,6 +1,6 @@ import go -import experimental.frameworks.CleverGo import TestUtilities.InlineExpectationsTest +import experimental.frameworks.CleverGo class Configuration extends TaintTracking::Configuration { Configuration() { this = "test-configuration" } diff --git a/ql/test/experimental/frameworks/CleverGo/UntrustedSources.go b/ql/test/experimental/frameworks/CleverGo/UntrustedSources.go index d4fe85ecf59..30b939ed902 100644 --- a/ql/test/experimental/frameworks/CleverGo/UntrustedSources.go +++ b/ql/test/experimental/frameworks/CleverGo/UntrustedSources.go @@ -5,7 +5,7 @@ package main import "clevergo.tech/clevergo" // Package clevergo.tech/clevergo@v0.5.2 -func UntrustedSources_ClevergoTechClevergov052() { +func UntrustedSources_ClevergoTechClevergoV052() { // Untrusted flow sources from method calls. { // Untrusted flow sources from method calls on clevergo.tech/clevergo.Context. diff --git a/ql/test/experimental/frameworks/CleverGo/UntrustedSources.ql b/ql/test/experimental/frameworks/CleverGo/UntrustedSources.ql index cb5b4f82cc2..e33df8fda49 100644 --- a/ql/test/experimental/frameworks/CleverGo/UntrustedSources.ql +++ b/ql/test/experimental/frameworks/CleverGo/UntrustedSources.ql @@ -1,6 +1,6 @@ import go -import experimental.frameworks.CleverGo import TestUtilities.InlineExpectationsTest +import experimental.frameworks.CleverGo class UntrustedFlowSourceTest extends InlineExpectationsTest { UntrustedFlowSourceTest() { this = "UntrustedFlowSourceTest" } From 110a3983c1a68733cab9b842ff009ea3ea677314 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Thu, 29 Apr 2021 16:00:05 +0200 Subject: [PATCH 09/10] Regenerate codeql: Refactor HTTP::HeaderWrite --- ql/src/experimental/frameworks/CleverGo.qll | 48 +++++++++++++-------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/ql/src/experimental/frameworks/CleverGo.qll b/ql/src/experimental/frameworks/CleverGo.qll index 5d9b2e19855..203f8ef42ed 100644 --- a/ql/src/experimental/frameworks/CleverGo.qll +++ b/ql/src/experimental/frameworks/CleverGo.qll @@ -390,37 +390,47 @@ private module CleverGo { } /** - * Models HTTP header writer models for package: clevergo.tech/clevergo@v0.5.2 + * Models HTTP header writers. + * The write is done with a call where you can set both the key and the value of the header. */ private class HeaderWrite extends HTTP::HeaderWrite::Range, DataFlow::CallNode { - string receiverName; - string methodName; + DataFlow::Node receiverNode; DataFlow::Node headerNameNode; DataFlow::Node headerValueNode; HeaderWrite() { - ( - // Type methods: - this = - any(Method m | m.hasQualifiedName(packagePath(), receiverName, methodName)).getACall() and - ( - // Receiver type: Context - receiverName = "Context" and - ( - // signature: func (*Context).SetHeader(key string, value string) - methodName = "SetHeader" and - headerNameNode = this.getArgument(0) and - headerValueNode = this.getArgument(1) - ) - ) - ) + setsHeaderDynamicKeyValue(_, _, this, headerNameNode, headerValueNode, receiverNode) } override DataFlow::Node getName() { result = headerNameNode } override DataFlow::Node getValue() { result = headerValueNode } - override HTTP::ResponseWriter getResponseWriter() { none() } + override HTTP::ResponseWriter getResponseWriter() { result.getANode() = receiverNode } + } + + // Holds for a call that sets a header with a key-value combination. + private predicate setsHeaderDynamicKeyValue( + string package, string receiverName, DataFlow::CallNode headerSetterCall, + DataFlow::Node headerNameNode, DataFlow::Node headerValueNode, DataFlow::Node receiverNode + ) { + exists(string methodName, Method met | + met.hasQualifiedName(package, receiverName, methodName) and + headerSetterCall = met.getACall() and + receiverNode = headerSetterCall.getReceiver() + | + package = packagePath() and + ( + // Receiver type: Context + receiverName = "Context" and + ( + // signature: func (*Context).SetHeader(key string, value string) + methodName = "SetHeader" and + headerNameNode = headerSetterCall.getArgument(0) and + headerValueNode = headerSetterCall.getArgument(1) + ) + ) + ) } /** From ea2909a362e907f6eee748e7270f9554883ceb16 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Fri, 30 Apr 2021 14:47:25 +0200 Subject: [PATCH 10/10] HTTP::HeaderWrite: Don't override `string getHeaderValue()` with `none()` --- ql/src/experimental/frameworks/CleverGo.qll | 2 -- 1 file changed, 2 deletions(-) diff --git a/ql/src/experimental/frameworks/CleverGo.qll b/ql/src/experimental/frameworks/CleverGo.qll index 203f8ef42ed..8aa25c377b1 100644 --- a/ql/src/experimental/frameworks/CleverGo.qll +++ b/ql/src/experimental/frameworks/CleverGo.qll @@ -499,8 +499,6 @@ private module CleverGo { override string getHeaderName() { result = "content-type" } - override string getHeaderValue() { none() } - override DataFlow::Node getName() { none() } override DataFlow::Node getValue() { result = contentTypeNode }