Make InsecureHostnameRegex check for rejecting handlers

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

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 |

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
+)

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

ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/vendor/github.com/elazarl/goproxy/.gitignore

@@ -0,0 +1,2 @@
+bin
+*.swp

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.

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)
+}

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