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/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