diff --git a/change-notes/2020-12-18-goproxy.md b/change-notes/2020-12-18-goproxy.md new file mode 100644 index 00000000000..e56d542a76b --- /dev/null +++ b/change-notes/2020-12-18-goproxy.md @@ -0,0 +1,5 @@ +lgtm,codescanning +* Added support for the `github.com/elazarl/goproxy` package. +* The query "Incomplete regular expression for hostnames" has been improved to recognize some cases + when the regexp in question is guarding an HTTP error response, which will lead to fewer false + positives. diff --git a/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql b/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql index 8aa7dae4c27..802f2cb61d5 100644 --- a/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql +++ b/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql @@ -30,6 +30,55 @@ predicate isIncompleteHostNameRegexpPattern(string pattern, string hostPart) { "(([():|?a-z0-9-]+(\\\\)?[.])?" + commonTLD() + ")" + ".*", 1) } +/** Holds if `b` sets the HTTP status code (represented by a pseudo-header named `status`) */ +predicate writesHttpError(ReachableBasicBlock b) { + forex(HTTP::HeaderWrite hw | + hw.getHeaderName() = "status" and hw.asInstruction().getBasicBlock() = b + | + exists(string code | code.matches("4__") or code.matches("5__") | + hw.definesHeader("status", code) + ) + ) +} + +/** Holds if starting from `block` a status code representing an error is certainly set. */ +predicate onlyErrors(BasicBlock block) { + writesHttpError(block) + or + forex(ReachableBasicBlock pred | pred = block.getAPredecessor() | onlyErrors(pred)) +} + +/** Gets a node that refers to a handler that is considered to return an HTTP error. */ +DataFlow::Node getASafeHandler() { + exists(Variable v | + v.hasQualifiedName(ElazarlGoproxy::packagePath(), ["AlwaysReject", "RejectConnect"]) + | + result = v.getARead() + ) + or + exists(Function f | + onlyErrors(f.getFuncDecl().(ControlFlow::Root).getExitNode().getBasicBlock()) + | + result = f.getARead() + ) +} + +/** Holds if `regexp` is used in a check before `handler` is called. */ +predicate regexpGuardsHandler(RegexpPattern regexp, HTTP::RequestHandler handler) { + handler.guardedBy(DataFlow::exprNode(regexp.getAUse().asExpr().getParent*())) +} + +/** Holds if `regexp` guards an HTTP error write. */ +predicate regexpGuardsError(RegexpPattern regexp) { + exists(ControlFlow::ConditionGuardNode cond, RegexpMatchFunction match, DataFlow::CallNode call | + call.getTarget() = match and + match.getRegexp(call) = regexp + | + cond.ensures(match.getResult().getNode(call).getASuccessor*(), true) and + cond.dominates(any(ReachableBasicBlock b | writesHttpError(b))) + ) +} + class Config extends DataFlow::Configuration { Config() { this = "IncompleteHostNameRegexp::Config" } @@ -47,7 +96,13 @@ class Config extends DataFlow::Configuration { override predicate isSource(DataFlow::Node source) { isSource(source, _) } - override predicate isSink(DataFlow::Node sink) { sink instanceof RegexpPattern } + override predicate isSink(DataFlow::Node sink) { + sink instanceof RegexpPattern and + forall(HTTP::RequestHandler handler | regexpGuardsHandler(sink, handler) | + not handler = getASafeHandler() + ) and + not regexpGuardsError(sink) + } } from Config c, DataFlow::PathNode source, DataFlow::PathNode sink, string hostPart diff --git a/ql/src/go.qll b/ql/src/go.qll index 85f99b3c774..393858594c0 100644 --- a/ql/src/go.qll +++ b/ql/src/go.qll @@ -34,6 +34,7 @@ import semmle.go.frameworks.BeegoOrm import semmle.go.frameworks.Chi import semmle.go.frameworks.Couchbase import semmle.go.frameworks.Echo +import semmle.go.frameworks.ElazarlGoproxy import semmle.go.frameworks.Email import semmle.go.frameworks.Encoding import semmle.go.frameworks.EvanphxJsonPatch diff --git a/ql/src/semmle/go/concepts/HTTP.qll b/ql/src/semmle/go/concepts/HTTP.qll index b4622b09515..3a5b9b8ff8b 100644 --- a/ql/src/semmle/go/concepts/HTTP.qll +++ b/ql/src/semmle/go/concepts/HTTP.qll @@ -64,10 +64,17 @@ module HTTP { /** Gets the (lower-case) name of a header set by this definition. */ string getHeaderName() { result = this.getName().getStringValue().toLowerCase() } + /** Gets the value of the header set by this definition. */ + string getHeaderValue() { + result = this.getValue().getStringValue() + or + result = this.getValue().getIntValue().toString() + } + /** Holds if this header write defines the header `header`. */ predicate definesHeader(string header, string value) { header = this.getHeaderName() and - value = this.getValue().getStringValue() + value = this.getHeaderValue() } /** @@ -101,6 +108,9 @@ module HTTP { /** Gets the (lower-case) name of a header set by this definition. */ string getHeaderName() { result = self.getHeaderName() } + /** Gets the value of the header set by this definition. */ + string getHeaderValue() { result = self.getHeaderValue() } + /** Holds if this header write defines the header `header`. */ predicate definesHeader(string header, string value) { self.definesHeader(header, value) } @@ -302,4 +312,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/ElazarlGoproxy.qll b/ql/src/semmle/go/frameworks/ElazarlGoproxy.qll new file mode 100644 index 00000000000..f6023ba12c7 --- /dev/null +++ b/ql/src/semmle/go/frameworks/ElazarlGoproxy.qll @@ -0,0 +1,135 @@ +/** + * Provides classes for working with concepts relating to the [github.com/elazarl/goproxy](https://pkg.go.dev/github.com/elazarl/goproxy) package. + */ + +import go + +/** + * Provides classes for working with concepts relating to the [github.com/elazarl/goproxy](https://pkg.go.dev/github.com/elazarl/goproxy) package. + */ +module ElazarlGoproxy { + /** Gets the package name. */ + bindingset[result] + string packagePath() { result = package("github.com/elazarl/goproxy", "") } + + private class NewResponse extends HTTP::HeaderWrite::Range, DataFlow::CallNode { + NewResponse() { this.getTarget().hasQualifiedName(packagePath(), "NewResponse") } + + override string getHeaderName() { this.definesHeader(result, _) } + + override string getHeaderValue() { this.definesHeader(_, result) } + + override DataFlow::Node getName() { none() } + + override DataFlow::Node getValue() { result = this.getArgument([1, 2]) } + + override predicate definesHeader(string header, string value) { + header = "status" and value = this.getArgument(2).getIntValue().toString() + or + header = "content-type" and value = this.getArgument(1).getStringValue() + } + + override HTTP::ResponseWriter getResponseWriter() { none() } + } + + /** A body argument to a `NewResponse` call. */ + private class NewResponseBody extends HTTP::ResponseBody::Range { + NewResponse call; + + NewResponseBody() { this = call.getArgument(3) } + + override DataFlow::Node getAContentTypeNode() { result = call.getArgument(1) } + + override HTTP::ResponseWriter getResponseWriter() { none() } + } + + private class TextResponse extends HTTP::HeaderWrite::Range, DataFlow::CallNode { + TextResponse() { this.getTarget().hasQualifiedName(packagePath(), "TextResponse") } + + override string getHeaderName() { this.definesHeader(result, _) } + + override string getHeaderValue() { this.definesHeader(_, result) } + + override DataFlow::Node getName() { none() } + + override DataFlow::Node getValue() { none() } + + override predicate definesHeader(string header, string value) { + header = "status" and value = "200" + or + header = "content-type" and value = "text/plain" + } + + override HTTP::ResponseWriter getResponseWriter() { none() } + } + + /** A body argument to a `TextResponse` call. */ + private class TextResponseBody extends HTTP::ResponseBody::Range, TextResponse { + TextResponse call; + + TextResponseBody() { this = call.getArgument(2) } + + override DataFlow::Node getAContentTypeNode() { result = call.getArgument(1) } + + override HTTP::ResponseWriter getResponseWriter() { none() } + } + + /** A handler attached to a goproxy proxy type. */ + private class ProxyHandler extends HTTP::RequestHandler::Range { + DataFlow::MethodCallNode handlerReg; + + ProxyHandler() { + handlerReg + .getTarget() + .hasQualifiedName(packagePath(), "ReqProxyConds", ["Do", "DoFunc", "HandleConnect"]) and + this = handlerReg.getArgument(0) + } + + override predicate guardedBy(DataFlow::Node check) { + // note OnResponse is not modeled, as that server responses are not currently considered untrusted input + exists(DataFlow::MethodCallNode onreqcall | + onreqcall.getTarget().hasQualifiedName(packagePath(), "ProxyHttpServer", "OnRequest") + | + handlerReg.getReceiver() = onreqcall.getASuccessor*() and + check = onreqcall.getArgument(0) + ) + } + } + + private class UserControlledRequestData extends UntrustedFlowSource::Range { + UserControlledRequestData() { + exists(DataFlow::FieldReadNode frn | this = frn | + // liberally consider ProxyCtx.UserData to be untrusted; it's a data field set by a request handler + frn.getField().hasQualifiedName(packagePath(), "ProxyCtx", "UserData") + ) + or + exists(DataFlow::MethodCallNode call | this = call | + call.getTarget().hasQualifiedName(packagePath(), "ProxyCtx", "Charset") + ) + } + } + + private class ProxyLog extends LoggerCall::Range, DataFlow::MethodCallNode { + ProxyLog() { this.getTarget().hasQualifiedName(packagePath(), "ProxyCtx", ["Logf", "Warnf"]) } + + override DataFlow::Node getAMessageComponent() { result = this.getAnArgument() } + } + + private class MethodModels extends TaintTracking::FunctionModel, Method { + FunctionInput inp; + FunctionOutput outp; + + MethodModels() { + // Methods: + // signature: func CertStorage.Fetch(hostname string, gen func() (*tls.Certificate, error)) (*tls.Certificate, error) + // + // `hostname` excluded because if the cert storage or generator function themselves have not + // been tainted, `hostname` would be unlikely to fetch user-controlled data + this.hasQualifiedName(packagePath(), "CertStorage", "Fetch") and + (inp.isReceiver() or inp.isParameter(1)) and + outp.isResult(0) + } + + override predicate hasTaintFlow(FunctionInput i, FunctionOutput o) { i = inp and o = outp } + } +} diff --git a/ql/src/semmle/go/frameworks/stdlib/NetHttp.qll b/ql/src/semmle/go/frameworks/stdlib/NetHttp.qll index 830e47f3c7e..575267c0f88 100644 --- a/ql/src/semmle/go/frameworks/stdlib/NetHttp.qll +++ b/ql/src/semmle/go/frameworks/stdlib/NetHttp.qll @@ -108,10 +108,6 @@ module NetHttp { override string getHeaderName() { result = "status" } - override predicate definesHeader(string header, string value) { - header = "status" and value = this.getValue().getIntValue().toString() - } - override DataFlow::Node getName() { none() } override DataFlow::Node getValue() { result = this.getArgument(0) } @@ -119,6 +115,18 @@ module NetHttp { override HTTP::ResponseWriter getResponseWriter() { result.getANode() = this.getReceiver() } } + private class ResponseErrorCall extends HTTP::HeaderWrite::Range, DataFlow::CallNode { + ResponseErrorCall() { this.getTarget().hasQualifiedName("net/http", "Error") } + + override string getHeaderName() { result = "status" } + + override DataFlow::Node getName() { none() } + + override DataFlow::Node getValue() { result = this.getArgument(2) } + + override HTTP::ResponseWriter getResponseWriter() { result.getANode() = this.getArgument(0) } + } + private class RequestBody extends HTTP::RequestBody::Range, DataFlow::ExprNode { RequestBody() { exists(Function newRequest | @@ -227,6 +235,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..7abf0a170c0 100644 --- a/ql/test/library-tests/semmle/go/concepts/HTTP/main.go +++ b/ql/test/library-tests/semmle/go/concepts/HTTP/main.go @@ -3,6 +3,7 @@ package main import ( "bytes" "fmt" + "html" "io" "io/ioutil" "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)) + }) } diff --git a/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/go.mod b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/go.mod new file mode 100644 index 00000000000..21f5e109021 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/go.mod @@ -0,0 +1,8 @@ +module main + +go 1.14 + +require ( + github.com/elazarl/goproxy v0.0.0-20201021153353-00ad82a08272 + github.com/github/depstubber v0.0.0-20201214172518-12c3da4b7c9d // indirect +) diff --git a/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/main.go b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/main.go new file mode 100644 index 00000000000..5c5f104e1b2 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/main.go @@ -0,0 +1,27 @@ +//go:generate depstubber -vendor github.com/elazarl/goproxy ProxyCtx NewResponse,TextResponse,AlwaysReject,ContentTypeText,ContentTypeHtml,ReqHostMatches + +package main + +import ( + "fmt" + "github.com/elazarl/goproxy" + "net/http" +) + +func handler(r *http.Request, ctx *goproxy.ProxyCtx) (*http.Request, *http.Response) { + data := ctx.UserData // $untrustedflowsource=selection of UserData + + // note no content type result here because we don't seem to extract the value of `ContentTypeHtml` + return r, goproxy.NewResponse(r, goproxy.ContentTypeHtml, http.StatusForbidden, fmt.Sprintf("Bad request: %v", data)) // $headerwrite=status:403 +} + +func handler1(r *http.Request, ctx *goproxy.ProxyCtx) (*http.Request, *http.Response) { + ctx.Logf("test") // $logger="test" + ctx.Warnf("test1") // $logger="test1" + + return r, goproxy.TextResponse(r, "Hello!") // $headerwrite=status:200 $headerwrite=content-type:text/plain +} + +func main() { + +} diff --git a/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/test.expected b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/test.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/test.ql b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/test.ql new file mode 100644 index 00000000000..cf7ff09fb3f --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/test.ql @@ -0,0 +1,46 @@ +import go +import TestUtilities.InlineExpectationsTest + +class UntrustedFlowSourceTest extends InlineExpectationsTest { + UntrustedFlowSourceTest() { this = "untrustedflowsource" } + + override string getARelevantTag() { result = "untrustedflowsource" } + + override predicate hasActualResult(string file, int line, string element, string tag, string value) { + tag = "untrustedflowsource" and + value = element and + exists(UntrustedFlowSource src | value = src.toString() | + src.hasLocationInfo(file, line, _, _, _) + ) + } +} + +class HeaderWriteTest extends InlineExpectationsTest { + HeaderWriteTest() { this = "headerwrite" } + + override string getARelevantTag() { result = "headerwrite" } + + override predicate hasActualResult(string file, int line, string element, string tag, string value) { + tag = "headerwrite" and + exists(HTTP::HeaderWrite hw, string name, string val | element = hw.toString() | + hw.definesHeader(name, val) and + value = name + ":" + val and + hw.hasLocationInfo(file, line, _, _, _) + ) + } +} + +class LoggerTest extends InlineExpectationsTest { + LoggerTest() { this = "LoggerTest" } + + override string getARelevantTag() { result = "logger" } + + override predicate hasActualResult(string file, int line, string element, string tag, string value) { + exists(LoggerCall log | + log.hasLocationInfo(file, line, _, _, _) and + element = log.toString() and + value = log.getAMessageComponent().toString() and + tag = "logger" + ) + } +} diff --git a/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/vendor/github.com/elazarl/goproxy/.gitignore b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/vendor/github.com/elazarl/goproxy/.gitignore new file mode 100644 index 00000000000..1005f6f1ecd --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/vendor/github.com/elazarl/goproxy/.gitignore @@ -0,0 +1,2 @@ +bin +*.swp diff --git a/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/vendor/github.com/elazarl/goproxy/LICENSE b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/vendor/github.com/elazarl/goproxy/LICENSE new file mode 100644 index 00000000000..2067e567c9f --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/vendor/github.com/elazarl/goproxy/LICENSE @@ -0,0 +1,27 @@ +Copyright (c) 2012 Elazar Leibovich. All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the following disclaimer +in the documentation and/or other materials provided with the +distribution. + * Neither the name of Elazar Leibovich. nor the names of its +contributors may be used to endorse or promote products derived from +this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/vendor/github.com/elazarl/goproxy/stub.go b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/vendor/github.com/elazarl/goproxy/stub.go new file mode 100644 index 00000000000..3d4882efc8b --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/vendor/github.com/elazarl/goproxy/stub.go @@ -0,0 +1,159 @@ +// Code generated by depstubber. DO NOT EDIT. +// This is a simple stub for github.com/elazarl/goproxy, strictly for use in testing. + +// See the LICENSE file for information about the licensing of the original library. +// Source: github.com/elazarl/goproxy (exports: ProxyCtx; functions: NewResponse,TextResponse,AlwaysReject,ContentTypeText,ContentTypeHtml,ReqHostMatches) + +// Package goproxy is a stub of github.com/elazarl/goproxy, generated by depstubber. +package goproxy + +import ( + tls "crypto/tls" + net "net" + http "net/http" + regexp "regexp" +) + +var AlwaysReject FuncHttpsHandler = nil + +type CertStorage interface { + Fetch(_ string, _ func() (*tls.Certificate, error)) (*tls.Certificate, error) +} + +type ConnectAction struct { + Action ConnectActionLiteral + Hijack func(*http.Request, net.Conn, *ProxyCtx) + TLSConfig func(string, *ProxyCtx) (*tls.Config, error) +} + +type ConnectActionLiteral int + +var ContentTypeHtml string = "" + +var ContentTypeText string = "" + +type FuncHttpsHandler func(string, *ProxyCtx) (*ConnectAction, string) + +func (_ FuncHttpsHandler) HandleConnect(_ string, _ *ProxyCtx) (*ConnectAction, string) { + return nil, "" +} + +type HttpsHandler interface { + HandleConnect(_ string, _ *ProxyCtx) (*ConnectAction, string) +} + +type Logger interface { + Printf(_ string, _ ...interface{}) +} + +func NewResponse(_ *http.Request, _ string, _ int, _ string) *http.Response { + return nil +} + +type ProxyConds struct{} + +func (_ *ProxyConds) Do(_ RespHandler) {} + +func (_ *ProxyConds) DoFunc(_ func(*http.Response, *ProxyCtx) *http.Response) {} + +type ProxyCtx struct { + Req *http.Request + Resp *http.Response + RoundTripper RoundTripper + Error error + UserData interface{} + Session int64 + Proxy *ProxyHttpServer +} + +func (_ *ProxyCtx) Charset() string { + return "" +} + +func (_ *ProxyCtx) Logf(_ string, _ ...interface{}) {} + +func (_ *ProxyCtx) RoundTrip(_ *http.Request) (*http.Response, error) { + return nil, nil +} + +func (_ *ProxyCtx) Warnf(_ string, _ ...interface{}) {} + +type ProxyHttpServer struct { + KeepDestinationHeaders bool + Verbose bool + Logger Logger + NonproxyHandler http.Handler + Tr *http.Transport + ConnectDial func(string, string) (net.Conn, error) + CertStore CertStorage + KeepHeader bool +} + +func (_ *ProxyHttpServer) NewConnectDialToProxy(_ string) func(string, string) (net.Conn, error) { + return nil +} + +func (_ *ProxyHttpServer) NewConnectDialToProxyWithHandler(_ string, _ func(*http.Request)) func(string, string) (net.Conn, error) { + return nil +} + +func (_ *ProxyHttpServer) OnRequest(_ ...ReqCondition) *ReqProxyConds { + return nil +} + +func (_ *ProxyHttpServer) OnResponse(_ ...RespCondition) *ProxyConds { + return nil +} + +func (_ *ProxyHttpServer) ServeHTTP(_ http.ResponseWriter, _ *http.Request) {} + +type ReqCondition interface { + HandleReq(_ *http.Request, _ *ProxyCtx) bool + HandleResp(_ *http.Response, _ *ProxyCtx) bool +} + +type ReqConditionFunc func(*http.Request, *ProxyCtx) bool + +func (_ ReqConditionFunc) HandleReq(_ *http.Request, _ *ProxyCtx) bool { + return false +} + +func (_ ReqConditionFunc) HandleResp(_ *http.Response, _ *ProxyCtx) bool { + return false +} + +type ReqHandler interface { + Handle(_ *http.Request, _ *ProxyCtx) (*http.Request, *http.Response) +} + +func ReqHostMatches(_ ...*regexp.Regexp) ReqConditionFunc { + return nil +} + +type ReqProxyConds struct{} + +func (_ *ReqProxyConds) Do(_ ReqHandler) {} + +func (_ *ReqProxyConds) DoFunc(_ func(*http.Request, *ProxyCtx) (*http.Request, *http.Response)) {} + +func (_ *ReqProxyConds) HandleConnect(_ HttpsHandler) {} + +func (_ *ReqProxyConds) HandleConnectFunc(_ func(string, *ProxyCtx) (*ConnectAction, string)) {} + +func (_ *ReqProxyConds) HijackConnect(_ func(*http.Request, net.Conn, *ProxyCtx)) {} + +type RespCondition interface { + HandleResp(_ *http.Response, _ *ProxyCtx) bool +} + +type RespHandler interface { + Handle(_ *http.Response, _ *ProxyCtx) *http.Response +} + +type RoundTripper interface { + RoundTrip(_ *http.Request, _ *ProxyCtx) (*http.Response, error) +} + +func TextResponse(_ *http.Request, _ string) *http.Response { + return nil +} diff --git a/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/vendor/modules.txt b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/vendor/modules.txt new file mode 100644 index 00000000000..bf027718a74 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/ElazarlGoproxy/vendor/modules.txt @@ -0,0 +1,5 @@ +# github.com/elazarl/goproxy v0.0.0-20201021153353-00ad82a08272 +## explicit +github.com/elazarl/goproxy +# github.com/github/depstubber v0.0.0-20201214172518-12c3da4b7c9d +## explicit diff --git a/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/IncompleteHostnameRegexp.expected b/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/IncompleteHostnameRegexp.expected index 73231e52340..bff95f0497d 100644 --- a/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/IncompleteHostnameRegexp.expected +++ b/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/IncompleteHostnameRegexp.expected @@ -3,7 +3,9 @@ edges nodes | IncompleteHostnameRegexp.go:11:8:11:36 | "^((www\|beta).)?example.com/" : string | semmle.label | "^((www\|beta).)?example.com/" : string | | IncompleteHostnameRegexp.go:12:38:12:39 | re | semmle.label | re | -| main.go:12:15:12:39 | `https://www.example.com` | semmle.label | `https://www.example.com` | +| main.go:39:60:39:79 | "^test2.github.com$" | semmle.label | "^test2.github.com$" | +| main.go:44:15:44:39 | `https://www.example.com` | semmle.label | `https://www.example.com` | #select | IncompleteHostnameRegexp.go:11:8:11:36 | "^((www\|beta).)?example.com/" : string | IncompleteHostnameRegexp.go:11:8:11:36 | "^((www\|beta).)?example.com/" : string | IncompleteHostnameRegexp.go:12:38:12:39 | re | This regular expression has an unescaped dot before ')?example.com', so it might match more hosts than expected when used $@. | IncompleteHostnameRegexp.go:12:38:12:39 | re | here | -| main.go:12:15:12:39 | `https://www.example.com` | main.go:12:15:12:39 | `https://www.example.com` | main.go:12:15:12:39 | `https://www.example.com` | This regular expression has an unescaped dot before 'example.com', so it might match more hosts than expected when used $@. | main.go:12:15:12:39 | `https://www.example.com` | here | +| main.go:39:60:39:79 | "^test2.github.com$" | main.go:39:60:39:79 | "^test2.github.com$" | main.go:39:60:39:79 | "^test2.github.com$" | This regular expression has an unescaped dot before 'github.com', so it might match more hosts than expected when used $@. | main.go:39:60:39:79 | "^test2.github.com$" | here | +| main.go:44:15:44:39 | `https://www.example.com` | main.go:44:15:44:39 | `https://www.example.com` | main.go:44:15:44:39 | `https://www.example.com` | This regular expression has an unescaped dot before 'example.com', so it might match more hosts than expected when used $@. | main.go:44:15:44:39 | `https://www.example.com` | here | diff --git a/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/go.mod b/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/go.mod new file mode 100644 index 00000000000..a77ab8418f8 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/go.mod @@ -0,0 +1,7 @@ +module main + +go 1.14 + +require ( + github.com/elazarl/goproxy v0.0.0-20201021153353-00ad82a08272 +) diff --git a/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/main.go b/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/main.go index 81123454971..fb1e5b77b51 100644 --- a/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/main.go +++ b/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/main.go @@ -1,13 +1,45 @@ +//go:generate depstubber -vendor github.com/elazarl/goproxy ProxyCtx NewProxyHttpServer,NewResponse,AlwaysReject,ContentTypeText,ReqHostMatches + package main import ( + "github.com/elazarl/goproxy" + "net/http" "regexp" + "time" ) func Match(notARegex string) bool { return notARegex != "" } +func stdlibreject(w http.ResponseWriter, r *http.Request) { + if matched, _ := regexp.MatchString(`test.github.com`, r.URL.String()); matched { + http.Error(w, "Bad request!", http.StatusBadRequest) + } +} + +func sometimesReject(req *http.Request, ctx *goproxy.ProxyCtx) (*http.Request, *http.Response) { + if h, _, _ := time.Now().Clock(); h >= 8 && h <= 17 { + return req, goproxy.NewResponse(req, goproxy.ContentTypeText, http.StatusForbidden, "Bad request!") + } + return req, nil +} + +func reject(req *http.Request, ctx *goproxy.ProxyCtx) (*http.Request, *http.Response) { + return req, goproxy.NewResponse(req, goproxy.ContentTypeText, http.StatusForbidden, "Bad request!") +} + +func proxy() { + proxy := goproxy.NewProxyHttpServer() + proxy.OnRequest(goproxy.ReqHostMatches(regexp.MustCompile("^test.github.com$"))). + HandleConnect(goproxy.AlwaysReject) // OK (rejecting all requests) + proxy.OnRequest(goproxy.ReqHostMatches(regexp.MustCompile("^test1.github.com$"))). + DoFunc(reject) // OK (rejecting all requests) + proxy.OnRequest(goproxy.ReqHostMatches(regexp.MustCompile("^test2.github.com$"))). + DoFunc(sometimesReject) // NOT OK (sometimes accepts requests) +} + func main() { regexp.Match(`https://www.example.com`, []byte("")) // NOT OK regexp.Match(`https://www\.example\.com`, []byte("")) // OK diff --git a/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/vendor/github.com/elazarl/goproxy/.gitignore b/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/vendor/github.com/elazarl/goproxy/.gitignore new file mode 100644 index 00000000000..1005f6f1ecd --- /dev/null +++ b/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/vendor/github.com/elazarl/goproxy/.gitignore @@ -0,0 +1,2 @@ +bin +*.swp diff --git a/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/vendor/github.com/elazarl/goproxy/LICENSE b/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/vendor/github.com/elazarl/goproxy/LICENSE new file mode 100644 index 00000000000..2067e567c9f --- /dev/null +++ b/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/vendor/github.com/elazarl/goproxy/LICENSE @@ -0,0 +1,27 @@ +Copyright (c) 2012 Elazar Leibovich. All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the following disclaimer +in the documentation and/or other materials provided with the +distribution. + * Neither the name of Elazar Leibovich. nor the names of its +contributors may be used to endorse or promote products derived from +this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/vendor/github.com/elazarl/goproxy/stub.go b/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/vendor/github.com/elazarl/goproxy/stub.go new file mode 100644 index 00000000000..62277c6e4ac --- /dev/null +++ b/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/vendor/github.com/elazarl/goproxy/stub.go @@ -0,0 +1,157 @@ +// Code generated by depstubber. DO NOT EDIT. +// This is a simple stub for github.com/elazarl/goproxy, strictly for use in testing. + +// See the LICENSE file for information about the licensing of the original library. +// Source: github.com/elazarl/goproxy (exports: ProxyCtx; functions: NewProxyHttpServer,NewResponse,AlwaysReject,ContentTypeText,ReqHostMatches) + +// Package goproxy is a stub of github.com/elazarl/goproxy, generated by depstubber. +package goproxy + +import ( + tls "crypto/tls" + net "net" + http "net/http" + regexp "regexp" +) + +var AlwaysReject FuncHttpsHandler = nil + +type CertStorage interface { + Fetch(_ string, _ func() (*tls.Certificate, error)) (*tls.Certificate, error) +} + +type ConnectAction struct { + Action ConnectActionLiteral + Hijack func(*http.Request, net.Conn, *ProxyCtx) + TLSConfig func(string, *ProxyCtx) (*tls.Config, error) +} + +type ConnectActionLiteral int + +var ContentTypeText string = "" + +type FuncHttpsHandler func(string, *ProxyCtx) (*ConnectAction, string) + +func (_ FuncHttpsHandler) HandleConnect(_ string, _ *ProxyCtx) (*ConnectAction, string) { + return nil, "" +} + +type HttpsHandler interface { + HandleConnect(_ string, _ *ProxyCtx) (*ConnectAction, string) +} + +type Logger interface { + Printf(_ string, _ ...interface{}) +} + +func NewProxyHttpServer() *ProxyHttpServer { + return nil +} + +func NewResponse(_ *http.Request, _ string, _ int, _ string) *http.Response { + return nil +} + +type ProxyConds struct{} + +func (_ *ProxyConds) Do(_ RespHandler) {} + +func (_ *ProxyConds) DoFunc(_ func(*http.Response, *ProxyCtx) *http.Response) {} + +type ProxyCtx struct { + Req *http.Request + Resp *http.Response + RoundTripper RoundTripper + Error error + UserData interface{} + Session int64 + Proxy *ProxyHttpServer +} + +func (_ *ProxyCtx) Charset() string { + return "" +} + +func (_ *ProxyCtx) Logf(_ string, _ ...interface{}) {} + +func (_ *ProxyCtx) RoundTrip(_ *http.Request) (*http.Response, error) { + return nil, nil +} + +func (_ *ProxyCtx) Warnf(_ string, _ ...interface{}) {} + +type ProxyHttpServer struct { + KeepDestinationHeaders bool + Verbose bool + Logger Logger + NonproxyHandler http.Handler + Tr *http.Transport + ConnectDial func(string, string) (net.Conn, error) + CertStore CertStorage + KeepHeader bool +} + +func (_ *ProxyHttpServer) NewConnectDialToProxy(_ string) func(string, string) (net.Conn, error) { + return nil +} + +func (_ *ProxyHttpServer) NewConnectDialToProxyWithHandler(_ string, _ func(*http.Request)) func(string, string) (net.Conn, error) { + return nil +} + +func (_ *ProxyHttpServer) OnRequest(_ ...ReqCondition) *ReqProxyConds { + return nil +} + +func (_ *ProxyHttpServer) OnResponse(_ ...RespCondition) *ProxyConds { + return nil +} + +func (_ *ProxyHttpServer) ServeHTTP(_ http.ResponseWriter, _ *http.Request) {} + +type ReqCondition interface { + HandleReq(_ *http.Request, _ *ProxyCtx) bool + HandleResp(_ *http.Response, _ *ProxyCtx) bool +} + +type ReqConditionFunc func(*http.Request, *ProxyCtx) bool + +func (_ ReqConditionFunc) HandleReq(_ *http.Request, _ *ProxyCtx) bool { + return false +} + +func (_ ReqConditionFunc) HandleResp(_ *http.Response, _ *ProxyCtx) bool { + return false +} + +type ReqHandler interface { + Handle(_ *http.Request, _ *ProxyCtx) (*http.Request, *http.Response) +} + +func ReqHostMatches(_ ...*regexp.Regexp) ReqConditionFunc { + return nil +} + +type ReqProxyConds struct{} + +func (_ *ReqProxyConds) Do(_ ReqHandler) {} + +func (_ *ReqProxyConds) DoFunc(_ func(*http.Request, *ProxyCtx) (*http.Request, *http.Response)) {} + +func (_ *ReqProxyConds) HandleConnect(_ HttpsHandler) {} + +func (_ *ReqProxyConds) HandleConnectFunc(_ func(string, *ProxyCtx) (*ConnectAction, string)) {} + +func (_ *ReqProxyConds) HijackConnect(_ func(*http.Request, net.Conn, *ProxyCtx)) {} + +type RespCondition interface { + HandleResp(_ *http.Response, _ *ProxyCtx) bool +} + +type RespHandler interface { + Handle(_ *http.Response, _ *ProxyCtx) *http.Response +} + +type RoundTripper interface { + RoundTrip(_ *http.Request, _ *ProxyCtx) (*http.Response, error) +} diff --git a/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/vendor/modules.txt b/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/vendor/modules.txt new file mode 100644 index 00000000000..bf027718a74 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/vendor/modules.txt @@ -0,0 +1,5 @@ +# github.com/elazarl/goproxy v0.0.0-20201021153353-00ad82a08272 +## explicit +github.com/elazarl/goproxy +# github.com/github/depstubber v0.0.0-20201214172518-12c3da4b7c9d +## explicit