hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-01-29 06:12:58 +01:00
Code Issues Packages Projects Releases Wiki Activity

Add support for html.Render method.

Browse Source 
This entails generalising Http::ResponseBody to account for any modelled function writing to a ResponseWriter.
This commit is contained in:
Chris Smowton
2020-10-21 15:27:43 +01:00
parent 02f353eabd
commit 3817ae80e5
6 changed files with 57 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
change-notes/2020-10-12-x-net-html.md
View File

@@ -1,2 +1,3 @@
lgtm,codescanning
* Added partial support for the `golang.org/x/net/html` package, modeling tainted data flow from a retrieved HTML document to its attributes and other data.
* Modeled more ways of writing data to an `net/http.ResponseWriter`. This may produce more results from queries such as `go/reflected-xss` which look for data flowing to an HTTP response.

11
ql/src/semmle/go/frameworks/XNetHtml.qll
View File

@@ -2,8 +2,9 @@
* Provides classes modeling security-relevant aspects of the `golang.org/x/net/html` subpackage.
*
* Currently we support the unmarshalling aspect of this package, conducting taint from an untrusted
* reader to an untrusted `Node` tree or `Tokenizer` instance. We do not yet model adding a child
* `Node` to a tree then calling `Render` yielding an untrustworthy string.
* reader to an untrusted `Node` tree or `Tokenizer` instance, as well as simple remarshalling of `Node`s
* that were already untrusted. We do not yet model adding a child `Node` to a tree then calling `Render`
* yielding an untrustworthy string.
*/
import go
@@ -34,12 +35,18 @@ module XNetHtml {
getName() = ["AppendChild", "InsertBefore"] and
input.isParameter(0) and
output.isReceiver()
or
getName() = "Render" and
input.isParameter(1) and
output.isParameter(0)
}
}
private class TokenizerMethodModels extends Method, TaintTracking::FunctionModel {
TokenizerMethodModels() { this.hasQualifiedName(packagePath(), "Tokenizer", _) }
// Note that `TagName` and the key part of `TagAttr` are not sources by default under the assumption
// that their character-set restrictions usually rule them out as useful attack routes.
override predicate hasTaintFlow(DataFlow::FunctionInput input, DataFlow::FunctionOutput output) {
getName() = ["Buffered", "Raw", "Text", "Token"] and input.isReceiver() and output.isResult(0)
or

36
ql/src/semmle/go/frameworks/stdlib/NetHttp.qll
View File

@@ -135,33 +135,31 @@ module NetHttp {
}
private class ResponseBody extends HTTP::ResponseBody::Range, DataFlow::ArgumentNode {
int arg;
DataFlow::Node responseWriter;
ResponseBody() {
exists(DataFlow::CallNode call |
// A direct call to ResponseWriter.Write, conveying taint from the argument to the receiver
call.getTarget().(Method).implements("net/http", "ResponseWriter", "Write") and
arg = 0
or
(
call.getTarget().hasQualifiedName("fmt", "Fprintf")
or
call.getTarget().hasQualifiedName("io", "WriteString")
) and
call.getArgument(0).getType().hasQualifiedName("net/http", "ResponseWriter") and
arg >= 1
this = call.getArgument(0) and
responseWriter = call.(DataFlow::MethodCallNode).getReceiver()
)
or
exists(
TaintTracking::FunctionModel model, FunctionOutput modelOutput, FunctionInput modelInput,
DataFlow::CallNode call
|
this = call.getArgument(arg)
// A modelled function conveying taint from some input to the response writer,
// e.g. `io.Copy(responseWriter, someTaintedReader)`
call = model.getACall() and
model.hasTaintFlow(modelInput, modelOutput) and
this = modelInput.getNode(call) and
responseWriter = modelOutput.getNode(call).(DataFlow::PostUpdateNode).getPreUpdateNode() and
responseWriter.getType().implements("net/http", "ResponseWriter")
)
}
override HTTP::ResponseWriter getResponseWriter() {
// the response writer is the receiver of this call
result.getANode() = this.getCall().(DataFlow::MethodCallNode).getReceiver()
or
// the response writer is an argument to Fprintf or WriteString
arg >= 1 and
result.getANode() = this.getCall().getArgument(0)
}
override HTTP::ResponseWriter getResponseWriter() { result.getANode() = responseWriter }
}
private class RedirectCall extends HTTP::Redirect::Range, DataFlow::CallNode {

42
ql/test/library-tests/semmle/go/frameworks/XNetHtml/ReflectedXss.expected
View File

@@ -5,8 +5,10 @@ edges
| test.go:14:42:14:47 | implicit dereference : Cookie | test.go:14:42:14:47 | implicit dereference : Cookie |
| test.go:16:24:16:35 | selection of Body : ReadCloser | test.go:17:15:17:31 | type conversion |
| test.go:16:24:16:35 | selection of Body : ReadCloser | test.go:17:22:17:25 | implicit dereference : Node |
| test.go:16:24:16:35 | selection of Body : ReadCloser | test.go:28:22:28:25 | node |
| test.go:17:22:17:25 | implicit dereference : Node | test.go:17:15:17:31 | type conversion |
| test.go:17:22:17:25 | implicit dereference : Node | test.go:17:22:17:25 | implicit dereference : Node |
| test.go:17:22:17:25 | implicit dereference : Node | test.go:28:22:28:25 | node |
| test.go:19:36:19:47 | selection of Body : ReadCloser | test.go:20:15:20:32 | type conversion |
| test.go:19:36:19:47 | selection of Body : ReadCloser | test.go:20:22:20:26 | implicit dereference : Node |
| test.go:20:22:20:26 | implicit dereference : Node | test.go:20:15:20:32 | type conversion |
@@ -19,13 +21,13 @@ edges
| test.go:25:45:25:56 | selection of Body : ReadCloser | test.go:26:22:26:30 | implicit dereference : Node |
| test.go:26:22:26:30 | implicit dereference : Node | test.go:26:15:26:36 | type conversion |
| test.go:26:22:26:30 | implicit dereference : Node | test.go:26:22:26:30 | implicit dereference : Node |
| test.go:28:33:28:44 | selection of Body : ReadCloser | test.go:29:15:29:34 | call to Buffered |
| test.go:28:33:28:44 | selection of Body : ReadCloser | test.go:30:15:30:29 | call to Raw |
| test.go:28:33:28:44 | selection of Body : ReadCloser | test.go:32:15:32:19 | value |
| test.go:28:33:28:44 | selection of Body : ReadCloser | test.go:33:15:33:30 | call to Text |
| test.go:28:33:28:44 | selection of Body : ReadCloser | test.go:34:15:34:44 | type conversion |
| test.go:28:33:28:44 | selection of Body : ReadCloser | test.go:34:22:34:38 | call to Token : Token |
| test.go:34:22:34:38 | call to Token : Token | test.go:34:15:34:44 | type conversion |
| test.go:30:33:30:44 | selection of Body : ReadCloser | test.go:31:15:31:34 | call to Buffered |
| test.go:30:33:30:44 | selection of Body : ReadCloser | test.go:32:15:32:29 | call to Raw |
| test.go:30:33:30:44 | selection of Body : ReadCloser | test.go:34:15:34:19 | value |
| test.go:30:33:30:44 | selection of Body : ReadCloser | test.go:35:15:35:30 | call to Text |
| test.go:30:33:30:44 | selection of Body : ReadCloser | test.go:36:15:36:44 | type conversion |
| test.go:30:33:30:44 | selection of Body : ReadCloser | test.go:36:22:36:38 | call to Token : Token |
| test.go:36:22:36:38 | call to Token : Token | test.go:36:15:36:44 | type conversion |
nodes
| test.go:10:15:10:42 | call to Cookie : tuple type | semmle.label | call to Cookie : tuple type |
| test.go:14:15:14:55 | type conversion | semmle.label | type conversion |
@@ -42,21 +44,23 @@ nodes
| test.go:25:45:25:56 | selection of Body : ReadCloser | semmle.label | selection of Body : ReadCloser |
| test.go:26:15:26:36 | type conversion | semmle.label | type conversion |
| test.go:26:22:26:30 | implicit dereference : Node | semmle.label | implicit dereference : Node |
| test.go:28:33:28:44 | selection of Body : ReadCloser | semmle.label | selection of Body : ReadCloser |
| test.go:29:15:29:34 | call to Buffered | semmle.label | call to Buffered |
| test.go:30:15:30:29 | call to Raw | semmle.label | call to Raw |
| test.go:32:15:32:19 | value | semmle.label | value |
| test.go:33:15:33:30 | call to Text | semmle.label | call to Text |
| test.go:34:15:34:44 | type conversion | semmle.label | type conversion |
| test.go:34:22:34:38 | call to Token : Token | semmle.label | call to Token : Token |
| test.go:28:22:28:25 | node | semmle.label | node |
| test.go:30:33:30:44 | selection of Body : ReadCloser | semmle.label | selection of Body : ReadCloser |
| test.go:31:15:31:34 | call to Buffered | semmle.label | call to Buffered |
| test.go:32:15:32:29 | call to Raw | semmle.label | call to Raw |
| test.go:34:15:34:19 | value | semmle.label | value |
| test.go:35:15:35:30 | call to Text | semmle.label | call to Text |
| test.go:36:15:36:44 | type conversion | semmle.label | type conversion |
| test.go:36:22:36:38 | call to Token : Token | semmle.label | call to Token : Token |
#select
| test.go:14:15:14:55 | type conversion | test.go:10:15:10:42 | call to Cookie : tuple type | test.go:14:15:14:55 | type conversion | Cross-site scripting vulnerability due to $@. | test.go:10:15:10:42 | call to Cookie | user-provided value |
| test.go:17:15:17:31 | type conversion | test.go:16:24:16:35 | selection of Body : ReadCloser | test.go:17:15:17:31 | type conversion | Cross-site scripting vulnerability due to $@. | test.go:16:24:16:35 | selection of Body | user-provided value |
| test.go:20:15:20:32 | type conversion | test.go:19:36:19:47 | selection of Body : ReadCloser | test.go:20:15:20:32 | type conversion | Cross-site scripting vulnerability due to $@. | test.go:19:36:19:47 | selection of Body | user-provided value |
| test.go:23:15:23:35 | type conversion | test.go:22:33:22:44 | selection of Body : ReadCloser | test.go:23:15:23:35 | type conversion | Cross-site scripting vulnerability due to $@. | test.go:22:33:22:44 | selection of Body | user-provided value |
| test.go:26:15:26:36 | type conversion | test.go:25:45:25:56 | selection of Body : ReadCloser | test.go:26:15:26:36 | type conversion | Cross-site scripting vulnerability due to $@. | test.go:25:45:25:56 | selection of Body | user-provided value |
| test.go:29:15:29:34 | call to Buffered | test.go:28:33:28:44 | selection of Body : ReadCloser | test.go:29:15:29:34 | call to Buffered | Cross-site scripting vulnerability due to $@. | test.go:28:33:28:44 | selection of Body | user-provided value |
| test.go:30:15:30:29 | call to Raw | test.go:28:33:28:44 | selection of Body : ReadCloser | test.go:30:15:30:29 | call to Raw | Cross-site scripting vulnerability due to $@. | test.go:28:33:28:44 | selection of Body | user-provided value |
| test.go:32:15:32:19 | value | test.go:28:33:28:44 | selection of Body : ReadCloser | test.go:32:15:32:19 | value | Cross-site scripting vulnerability due to $@. | test.go:28:33:28:44 | selection of Body | user-provided value |
| test.go:33:15:33:30 | call to Text | test.go:28:33:28:44 | selection of Body : ReadCloser | test.go:33:15:33:30 | call to Text | Cross-site scripting vulnerability due to $@. | test.go:28:33:28:44 | selection of Body | user-provided value |
| test.go:34:15:34:44 | type conversion | test.go:28:33:28:44 | selection of Body : ReadCloser | test.go:34:15:34:44 | type conversion | Cross-site scripting vulnerability due to $@. | test.go:28:33:28:44 | selection of Body | user-provided value |
| test.go:28:22:28:25 | node | test.go:16:24:16:35 | selection of Body : ReadCloser | test.go:28:22:28:25 | node | Cross-site scripting vulnerability due to $@. | test.go:16:24:16:35 | selection of Body | user-provided value |
| test.go:31:15:31:34 | call to Buffered | test.go:30:33:30:44 | selection of Body : ReadCloser | test.go:31:15:31:34 | call to Buffered | Cross-site scripting vulnerability due to $@. | test.go:30:33:30:44 | selection of Body | user-provided value |
| test.go:32:15:32:29 | call to Raw | test.go:30:33:30:44 | selection of Body : ReadCloser | test.go:32:15:32:29 | call to Raw | Cross-site scripting vulnerability due to $@. | test.go:30:33:30:44 | selection of Body | user-provided value |
| test.go:34:15:34:19 | value | test.go:30:33:30:44 | selection of Body : ReadCloser | test.go:34:15:34:19 | value | Cross-site scripting vulnerability due to $@. | test.go:30:33:30:44 | selection of Body | user-provided value |
| test.go:35:15:35:30 | call to Text | test.go:30:33:30:44 | selection of Body : ReadCloser | test.go:35:15:35:30 | call to Text | Cross-site scripting vulnerability due to $@. | test.go:30:33:30:44 | selection of Body | user-provided value |
| test.go:36:15:36:44 | type conversion | test.go:30:33:30:44 | selection of Body : ReadCloser | test.go:36:15:36:44 | type conversion | Cross-site scripting vulnerability due to $@. | test.go:30:33:30:44 | selection of Body | user-provided value |

2
ql/test/library-tests/semmle/go/frameworks/XNetHtml/test.go
View File

@@ -25,6 +25,8 @@ func test(request *http.Request, writer http.ResponseWriter) {
nodes2, _ := html.ParseFragmentWithOptions(request.Body, nil)
writer.Write([]byte(nodes2[0].Data)) // BAD: writing unescaped HTML data
html.Render(writer, node) // BAD: rendering untrusted HTML to `writer`
tokenizer := html.NewTokenizer(request.Body)
writer.Write(tokenizer.Buffered()) // BAD: writing unescaped HTML data
writer.Write(tokenizer.Raw()) // BAD: writing unescaped HTML data

6
ql/test/library-tests/semmle/go/frameworks/XNetHtml/vendor/golang.org/x/net/html/stub.go generated vendored
View File

@@ -2,7 +2,7 @@
// This is a simple stub for golang.org/x/net/html, strictly for use in testing.
// See the LICENSE file for information about the licensing of the original library.
// Source: golang.org/x/net/html (exports: Node,Token,Tokenizer; functions: EscapeString,UnescapeString,Parse,ParseWithOptions,ParseFragment,ParseFragmentWithOptions,NewTokenizer)
// Source: golang.org/x/net/html (exports: Node,Token,Tokenizer; functions: EscapeString,UnescapeString,Parse,ParseWithOptions,ParseFragment,ParseFragmentWithOptions,NewTokenizer,Render)
// Package html is a stub of golang.org/x/net/html, generated by depstubber.
package html
@@ -124,3 +124,7 @@ func UnescapeString(_ string) string {
func NewTokenizer(r io.Reader) *Tokenizer {
return nil
}
func Render(w io.Writer, n *Node) error {
return nil
}