From 39c33c5db148fffb63f86159e048f2a02fa8d26d Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Mon, 7 Dec 2020 02:10:04 -0800 Subject: [PATCH] Add HTTP handler concept --- ql/src/semmle/go/concepts/HTTP.qll | 29 +++++++++++++++++++ .../semmle/go/frameworks/stdlib/NetHttp.qll | 14 +++++++++ .../semmle/go/concepts/HTTP/Handler.expected | 0 .../semmle/go/concepts/HTTP/Handler.ql | 18 ++++++++++++ .../semmle/go/concepts/HTTP/Header.expected | 20 ++++++------- .../go/concepts/HTTP/RequestBody.expected | 4 +-- .../go/concepts/HTTP/ResponseBody.expected | 6 ++-- .../HTTP/UntrustedFlowSources.expected | 19 ++++++------ .../semmle/go/concepts/HTTP/main.go | 7 +++++ 9 files changed, 94 insertions(+), 23 deletions(-) create mode 100644 ql/test/library-tests/semmle/go/concepts/HTTP/Handler.expected create mode 100644 ql/test/library-tests/semmle/go/concepts/HTTP/Handler.ql diff --git a/ql/src/semmle/go/concepts/HTTP.qll b/ql/src/semmle/go/concepts/HTTP.qll index b4622b09515..450ed88997a 100644 --- a/ql/src/semmle/go/concepts/HTTP.qll +++ b/ql/src/semmle/go/concepts/HTTP.qll @@ -302,4 +302,33 @@ module HTTP { /** Gets the response writer that this redirect is sent on, if any. */ ResponseWriter getResponseWriter() { result = self.getResponseWriter() } } + + /** Provides a class for modeling new HTTP handler APIs. */ + module RequestHandler { + /** + * An HTTP request handler. + * + * Extend this class to model new APIs. If you want to refine existing API models, + * extend `HTTP::RequestHandler` instead. + */ + abstract class Range extends DataFlow::Node { + /** Gets a node that is used in a check that is tested before this handler is run. */ + abstract predicate guardedBy(DataFlow::Node check); + } + } + + /** + * An HTTP request handler. + * + * Extend this class to refine existing API models. If you want to model new APIs, + * extend `HTTP::RequestHandler::Range` instead. + */ + class RequestHandler extends DataFlow::Node { + RequestHandler::Range self; + + RequestHandler() { this = self } + + /** Gets a node that is used in a check that is tested before this handler is run. */ + predicate guardedBy(DataFlow::Node check) { self.guardedBy(check) } + } } diff --git a/ql/src/semmle/go/frameworks/stdlib/NetHttp.qll b/ql/src/semmle/go/frameworks/stdlib/NetHttp.qll index 830e47f3c7e..5620dc8c84f 100644 --- a/ql/src/semmle/go/frameworks/stdlib/NetHttp.qll +++ b/ql/src/semmle/go/frameworks/stdlib/NetHttp.qll @@ -227,6 +227,20 @@ module NetHttp { } } + private class Handler extends HTTP::RequestHandler::Range { + DataFlow::CallNode handlerReg; + + Handler() { + exists(Function regFn | regFn = handlerReg.getTarget() | + regFn.hasQualifiedName("net/http", ["Handle", "HandleFunc"]) or + regFn.(Method).hasQualifiedName("net/http", "ServeMux", ["Handle", "HandleFunc"]) + ) and + this = handlerReg.getArgument(1) + } + + override predicate guardedBy(DataFlow::Node check) { check = handlerReg.getArgument(0) } + } + private class FunctionModels extends TaintTracking::FunctionModel { FunctionInput inp; FunctionOutput outp; diff --git a/ql/test/library-tests/semmle/go/concepts/HTTP/Handler.expected b/ql/test/library-tests/semmle/go/concepts/HTTP/Handler.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/ql/test/library-tests/semmle/go/concepts/HTTP/Handler.ql b/ql/test/library-tests/semmle/go/concepts/HTTP/Handler.ql new file mode 100644 index 00000000000..2ab16946cf9 --- /dev/null +++ b/ql/test/library-tests/semmle/go/concepts/HTTP/Handler.ql @@ -0,0 +1,18 @@ +import go +import TestUtilities.InlineExpectationsTest + +class HttpHandler extends InlineExpectationsTest { + HttpHandler() { this = "httphandler" } + + override string getARelevantTag() { result = "handler" } + + override predicate hasActualResult(string file, int line, string element, string tag, string value) { + tag = "handler" and + exists(HTTP::RequestHandler h, DataFlow::Node check | + element = h.toString() and value = check.toString() + | + h.hasLocationInfo(file, line, _, _, _) and + h.guardedBy(check) + ) + } +} diff --git a/ql/test/library-tests/semmle/go/concepts/HTTP/Header.expected b/ql/test/library-tests/semmle/go/concepts/HTTP/Header.expected index 1c57549d95b..e9d4558e705 100644 --- a/ql/test/library-tests/semmle/go/concepts/HTTP/Header.expected +++ b/ql/test/library-tests/semmle/go/concepts/HTTP/Header.expected @@ -1,10 +1,10 @@ -| main.go:29:2:29:19 | call to WriteHeader | n/a | 418 | status | 418 | -| main.go:31:2:31:51 | call to Set | "Authorization" | "Basic example:example" | authorization | Basic example:example | -| main.go:32:2:32:26 | call to Add | "Age" | "342232" | age | 342232 | -| main.go:34:2:34:55 | call to Add | server | call to Sprintf | n/a | n/a | -| main.go:35:2:35:45 | call to Set | LOC_HEADER | ...+... | n/a | n/a | -| main.go:36:2:36:5 | head | "Unknown-Header" | slice literal | n/a | n/a | -| main.go:48:2:48:43 | call to Add | "Not-A-Response" | "Header" | not-a-response | Header | -| main.go:49:2:49:42 | call to Set | "Accept" | "nota/response" | accept | nota/response | -| main.go:50:2:50:11 | selection of Header | "Accept-Charset" | slice literal | n/a | n/a | -| main.go:57:2:57:42 | call to Set | "This-Makes" | "No sense" | this-makes | No sense | +| main.go:30:2:30:19 | call to WriteHeader | n/a | 418 | status | 418 | +| main.go:32:2:32:51 | call to Set | "Authorization" | "Basic example:example" | authorization | Basic example:example | +| main.go:33:2:33:26 | call to Add | "Age" | "342232" | age | 342232 | +| main.go:35:2:35:55 | call to Add | server | call to Sprintf | n/a | n/a | +| main.go:36:2:36:45 | call to Set | LOC_HEADER | ...+... | n/a | n/a | +| main.go:37:2:37:5 | head | "Unknown-Header" | slice literal | n/a | n/a | +| main.go:49:2:49:43 | call to Add | "Not-A-Response" | "Header" | not-a-response | Header | +| main.go:50:2:50:42 | call to Set | "Accept" | "nota/response" | accept | nota/response | +| main.go:51:2:51:11 | selection of Header | "Accept-Charset" | slice literal | n/a | n/a | +| main.go:58:2:58:42 | call to Set | "This-Makes" | "No sense" | this-makes | No sense | diff --git a/ql/test/library-tests/semmle/go/concepts/HTTP/RequestBody.expected b/ql/test/library-tests/semmle/go/concepts/HTTP/RequestBody.expected index 1d2ac44dc6a..3a835aec7d6 100644 --- a/ql/test/library-tests/semmle/go/concepts/HTTP/RequestBody.expected +++ b/ql/test/library-tests/semmle/go/concepts/HTTP/RequestBody.expected @@ -1,2 +1,2 @@ -| main.go:46:57:46:59 | nil | -| main.go:52:13:52:34 | call to NopCloser | +| main.go:47:57:47:59 | nil | +| main.go:53:13:53:34 | call to NopCloser | diff --git a/ql/test/library-tests/semmle/go/concepts/HTTP/ResponseBody.expected b/ql/test/library-tests/semmle/go/concepts/HTTP/ResponseBody.expected index f36de03103b..7cba3845c39 100644 --- a/ql/test/library-tests/semmle/go/concepts/HTTP/ResponseBody.expected +++ b/ql/test/library-tests/semmle/go/concepts/HTTP/ResponseBody.expected @@ -1,2 +1,4 @@ -| main.go:38:10:38:40 | type conversion | -| main.go:40:20:40:30 | "Body text" | +| main.go:39:10:39:40 | type conversion | +| main.go:41:20:41:30 | "Body text" | +| main.go:63:18:63:28 | "Hello, %q" | +| main.go:63:31:63:59 | call to EscapeString | diff --git a/ql/test/library-tests/semmle/go/concepts/HTTP/UntrustedFlowSources.expected b/ql/test/library-tests/semmle/go/concepts/HTTP/UntrustedFlowSources.expected index f2f61551504..bf47ab17a58 100644 --- a/ql/test/library-tests/semmle/go/concepts/HTTP/UntrustedFlowSources.expected +++ b/ql/test/library-tests/semmle/go/concepts/HTTP/UntrustedFlowSources.expected @@ -1,14 +1,15 @@ -| main.go:16:10:16:15 | selection of Body | -| main.go:17:10:17:15 | selection of Form | -| main.go:18:10:18:17 | selection of Header | -| main.go:19:10:19:14 | selection of URL | -| main.go:23:10:23:16 | selection of Body | -| main.go:24:10:24:16 | selection of Form | -| main.go:25:10:25:18 | selection of Header | -| main.go:26:10:26:15 | selection of URL | -| main.go:48:2:48:11 | selection of Header | +| main.go:17:10:17:15 | selection of Body | +| main.go:18:10:18:15 | selection of Form | +| main.go:19:10:19:17 | selection of Header | +| main.go:20:10:20:14 | selection of URL | +| main.go:24:10:24:16 | selection of Body | +| main.go:25:10:25:16 | selection of Form | +| main.go:26:10:26:18 | selection of Header | +| main.go:27:10:27:15 | selection of URL | | main.go:49:2:49:11 | selection of Header | | main.go:50:2:50:11 | selection of Header | +| main.go:51:2:51:11 | selection of Header | +| main.go:63:49:63:53 | selection of URL | | server.go:8:6:8:13 | selection of Header | | server.go:9:6:9:13 | selection of Header | | server.go:10:6:10:13 | selection of Header | diff --git a/ql/test/library-tests/semmle/go/concepts/HTTP/main.go b/ql/test/library-tests/semmle/go/concepts/HTTP/main.go index cb8e8a85b43..a3c4e9a7a0e 100644 --- a/ql/test/library-tests/semmle/go/concepts/HTTP/main.go +++ b/ql/test/library-tests/semmle/go/concepts/HTTP/main.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "io/ioutil" + "net/html" "net/http" ) @@ -55,4 +56,10 @@ func main() { resp, _ := http.Get("https://example.com") resp.Header.Set("This-Makes", "No sense") + + http.HandleFunc("/foo", handler) // $handler="/foo" + + http.HandleFunc("/bar", func(w http.ResponseWriter, r *http.Request) { // $handler="/bar" + fmt.Fprintf(w, "Hello, %q", html.EscapeString(r.URL.Path)) + }) }