Remediate CodeQL Issues (#12)

.github/workflows/codeql.yml

@@ -2,8 +2,12 @@ name: "CodeQL"
 
 on:
   workflow_dispatch: {}
-  schedule:
-  - cron: '40 19 * * 2'
+  #push:
+  #  branches: [ main ]
+  #pull_request:
+  #  branches: [ main ]
+  #schedule:
+  #- cron: '40 19 * * 2'
 
 jobs:
   analyze:
@@ -38,13 +42,8 @@ jobs:
     - name: Perform CodeQL Analysis
       uses: github/codeql-action/analyze@v2
       with:
-        upload: false
-
-    - name: Debug
-      run: |
-        pwd
-        find "${RUNNER_WORKSPACE}/"
-        env | grep "go-authcrunch"
+        #  upload: false
+        wait-for-processing: true
 
     - name: Upload CodeQL Analysis
       uses: actions/upload-artifact@v3

Makefile

@@ -83,9 +83,10 @@ qtest: covdir
 	@echo "Perform quick tests ..."
 	@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out -run TestNewConfig ./*.go
 	@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out -run TestNewServer ./*.go
-	@time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out ./pkg/registry/...
+	@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out ./pkg/registry/...
+	@time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out ./pkg/waf/...
 	@#time richgo test -v -coverprofile=.coverage/coverage.out internal/tag/*.go
-	@### time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out -run TestAuthorize ./pkg/authz/validator/...
+	@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out -run TestAuthorize ./pkg/authz/validator/...
 	@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out  -run TestAddProviders ./pkg/messaging/...
 	@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out ./pkg/messaging/...
 	@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out ./pkg/credentials/...

assets/scripts/run_codeql_scan.sh

@@ -0,0 +1,33 @@
+#!/bin/bash
+set -e
+
+export CODEQL_SCAN_ID=$(date "+%Y%m%d_%H%M%S")
+
+printf "CodeQL Scan ID: ${CODEQL_SCAN_ID}\n";
+
+mkdir -p $HOME/.local/codeql/databases
+cd $HOME/.local/codeql/databases
+codeql database create \
+  --language="go" \
+  --source-root="${GOPATH}/src/github.com/greenpau/go-authcrunch" \
+  -- ./go-authcrunch-${CODEQL_SCAN_ID}
+
+cd $HOME/.local/codeql
+codeql database run-queries --ram=5922 --threads=2 --verbose \
+  --additional-packs . \
+  -- ./databases/go-authcrunch-${CODEQL_SCAN_ID} \
+  ./queries-go/ql/src/codeql-suites/go-code-scanning.qls
+
+cd $HOME/.local/codeql
+mkdir -p ./results/go-authcrunch
+codeql database interpret-results --format csv \
+  --output ./results/go-authcrunch/codeql_results_${CODEQL_SCAN_ID}.csv \
+  -- ./databases/go-authcrunch-${CODEQL_SCAN_ID}
+
+printf "CodeQL Scan Results (CSV): "`pwd`"/results/go-authcrunch/codeql_results_${CODEQL_SCAN_ID}.csv\n"
+
+codeql database interpret-results --format sarif-latest \
+  --output ./results/go-authcrunch/codeql_results_${CODEQL_SCAN_ID}_sarif.json \
+  -- ./databases/go-authcrunch-${CODEQL_SCAN_ID}
+
+printf "CodeQL Scan Results (SARIF): "`pwd`"/results/go-authcrunch/codeql_results_${CODEQL_SCAN_ID}_sarif.json\n"

internal/tests/eval.go

@@ -26,7 +26,7 @@ import (
 )
 
 var (
-	pr = regexp.MustCompile(".*(github.com/greenpau/go-authcrunch/.*)$")
+	pr = regexp.MustCompile(`.*(github\\.com/greenpau/go-authcrunch/.*)$`)
 )
 
 // EvalErr evaluates whether there is an error. If there is, was it the

pkg/authn/handle_register.go

@@ -273,7 +273,18 @@ func (p *Portal) handleHTTPRegisterRequest(ctx context.Context, w http.ResponseW
 				"username":          userHandle,
 				"email":             userMail,
 			}
-			regData["registration_url"] = getCurrentURL(r, "/register")
+
+			regURL, err := addrutil.GetCurrentURLWithSuffix(r, "/register")
+			if err != nil {
+				p.logger.Warn(
+					"Detected malformed request headers",
+					zap.String("session_id", rr.Upstream.SessionID),
+					zap.String("request_id", rr.ID),
+					zap.Error(err),
+				)
+			}
+			regData["registration_url"] = regURL
+
 			regData["src_ip"] = addrutil.GetSourceAddress(r)
 			regData["src_conn_ip"] = addrutil.GetSourceConnAddress(r)
 			regData["timestamp"] = time.Now().UTC().Format(time.UnixDate)
@@ -421,7 +432,18 @@ func (p *Portal) handleHTTPRegisterAckRequest(ctx context.Context, w http.Respon
 		"username":        req.User.Username,
 		"email":           req.User.Email,
 	}
-	regData["registration_url"] = getCurrentURL(r, "/register")
+
+	regURL, err := addrutil.GetCurrentURLWithSuffix(r, "/register")
+	if err != nil {
+		p.logger.Warn(
+			"Detected malformed request headers",
+			zap.String("session_id", rr.Upstream.SessionID),
+			zap.String("request_id", rr.ID),
+			zap.Error(err),
+		)
+	}
+	regData["registration_url"] = regURL
+
 	regData["src_ip"] = addrutil.GetSourceAddress(r)
 	regData["src_conn_ip"] = addrutil.GetSourceConnAddress(r)
 	regData["timestamp"] = time.Now().UTC().Format(time.UnixDate)
@@ -440,45 +462,3 @@ func (p *Portal) handleHTTPRegisterAckRequest(ctx context.Context, w http.Respon
 	reg.view = "acked"
 	return p.handleHTTPRegisterScreenWithMessage(ctx, w, r, rr, reg)
 }
-
-func getCurrentURL(r *http.Request, suffix string) string {
-	h := r.Header.Get("X-Forwarded-Host")
-	if h == "" {
-		h = r.Host
-	}
-	p := r.Header.Get("X-Forwarded-Proto")
-	if p == "" {
-		if r.TLS == nil {
-			p = "http"
-		} else {
-			p = "https"
-		}
-	}
-	port := r.Header.Get("X-Forwarded-Port")
-
-	u := p + "://" + h
-
-	if port != "" {
-		switch port {
-		case "443":
-			if p != "https" {
-				u += ":" + port
-			}
-		case "80":
-			if p != "http" {
-				u += ":" + port
-			}
-		default:
-			u += ":" + port
-		}
-	}
-	if suffix != "" {
-		i := strings.Index(r.RequestURI, suffix)
-		if i < 0 {
-			return u + r.RequestURI
-		}
-		return u + r.RequestURI[:i] + suffix
-	}
-
-	return u + r.RequestURI
-}

pkg/authz/handlers/redirect.go

@@ -16,6 +16,7 @@ package handlers
 
 import (
 	"github.com/greenpau/go-authcrunch/pkg/requests"
+	addrutil "github.com/greenpau/go-authcrunch/pkg/util/addr"
 	"html/template"
 	"net/http"
 	"net/url"
@@ -101,48 +102,23 @@ func configureRedirect(w http.ResponseWriter, r *http.Request, rr *requests.Auth
 		return
 	}
 
-	rr.Redirect.Enabled = true
-
 	if rr.Redirect.QueryDisabled {
 		return
 	}
 
-	rr.Redirect.Separator = "?"
-	rr.Redirect.URL = r.RequestURI
-
-	if strings.HasPrefix(rr.Redirect.URL, "/") {
-		redirHost := r.Header.Get("X-Forwarded-Host")
-		if redirHost == "" {
-			redirHost = r.Host
-		}
-		redirProto := r.Header.Get("X-Forwarded-Proto")
-		if redirProto == "" {
-			if r.TLS == nil {
-				redirProto = "http"
-			} else {
-				redirProto = "https"
-			}
+	if strings.HasPrefix(r.RequestURI, "/") {
+		u, err := addrutil.GetCurrentURLWithSuffix(r, "")
+		if err != nil {
+			return
 		}
-		redirPort := r.Header.Get("X-Forwarded-Port")
-
-		redirectBaseURL := redirProto + "://" + redirHost
-		if redirPort != "" {
-			switch redirPort {
-			case "443":
-				if redirProto != "https" {
-					redirectBaseURL += ":" + redirPort
-				}
-			case "80":
-				if redirProto != "http" {
-					redirectBaseURL += ":" + redirPort
-				}
-			default:
-				redirectBaseURL += ":" + redirPort
-			}
-		}
-		rr.Redirect.URL = redirectBaseURL + r.RequestURI
+		rr.Redirect.URL = u
+	} else {
+		rr.Redirect.URL = r.RequestURI
 	}
 
+	rr.Redirect.Enabled = true
+	rr.Redirect.Separator = "?"
+
 	if strings.Contains(rr.Redirect.AuthURL, "?") {
 		rr.Redirect.Separator = "&"
 	}

pkg/errors/common.go

@@ -84,7 +84,7 @@ const (
 	ErrAccessNotAllowed                   StandardError = "user role is valid, but not allowed by access list"
 	ErrAccessNotAllowedByPathACL          StandardError = "user role is valid, but not allowed by path access list"
 	ErrSourceAddressNotFound              StandardError = "source ip validation is enabled, but no ip address claim found"
-	ErrSourceAddressMismatch              StandardError = "source ip address mismatch between the claim %s and request %s"
+	ErrSourceAddressMismatch              StandardError = "source ip address mismatch between the claim %q and request %q"
 	ErrNoParsedClaims                     StandardError = "failed to extract claims"
 	ErrNoTokenFound                       StandardError = "no token found"
 	ErrInvalidParsedClaims                StandardError = "failed to extract claims: %s"

pkg/identity/database.go

@@ -62,8 +62,8 @@ func init() {
 	app.Description = "authdb"
 	app.Documentation = "https://github.com/greenpau/go-authcrunch"
 	app.SetVersion(appVersion, "1.0.23")
-	app.SetGitBranch(gitBranch, "main")
-	app.SetGitCommit(gitCommit, "v1.0.22-8-gd4c2857")
+	app.SetGitBranch(gitBranch, "codeqlfix")
+	app.SetGitCommit(gitCommit, "v1.0.23-16-g07a0218")
 	app.SetBuildUser(buildUser, "")
 	app.SetBuildDate(buildDate, "")
 }

pkg/util/addr/utils.go

@@ -15,35 +15,31 @@
 package addr
 
 import (
+	"fmt"
+	"github.com/greenpau/go-authcrunch/pkg/waf"
 	"net/http"
 	"strings"
 )
 
+const malformedURLStr = "malformed-url"
+
 // GetSourceHost returns the host or host:port of the request.
 func GetSourceHost(r *http.Request) string {
 	h := r.Header.Get("X-Forwarded-Host")
-	if h != "" {
-		return h
+	if !waf.IsMalformedForwardedHost(h, 2, 255) {
+		if h != "" {
+			return h
+		}
 	}
 	return r.Host
 }
 
-// GetSourceAddress returns the IP address of the request.
-func GetSourceAddress(r *http.Request) string {
-	var addr string
-	if r.Header.Get("X-Real-Ip") != "" {
-		addr = r.Header.Get("X-Real-Ip")
-	} else {
-		if r.Header.Get("X-Forwarded-For") != "" {
-			addr = r.Header.Get("X-Forwarded-For")
-		} else {
-			addr = r.RemoteAddr
-		}
-	}
+func parseSourceAddress(addr string) string {
 	if strings.Contains(addr, ",") {
 		addr = strings.TrimSpace(addr)
 		addr = strings.SplitN(addr, ",", 2)[0]
 	}
+
 	switch {
 	case strings.Contains(addr, "["):
 		// Handle IPv6 "[host]:port" address.
@@ -52,6 +48,7 @@ func GetSourceAddress(r *http.Request) string {
 		// Handle IPv6 address.
 		return addr
 	}
+
 	if strings.Contains(addr, ":") {
 		parts := strings.Split(addr, ":")
 		if len(parts) > 2 {
@@ -60,9 +57,25 @@ func GetSourceAddress(r *http.Request) string {
 		}
 		return parts[0]
 	}
+
 	return addr
 }
 
+// GetSourceAddress returns the IP address of the request.
+func GetSourceAddress(r *http.Request) string {
+	if r.Header.Get("X-Real-Ip") != "" {
+		if !waf.IsMalformedRealIP(r.Header.Get("X-Real-Ip"), 7, 255) {
+			return parseSourceAddress(r.Header.Get("X-Real-Ip"))
+		}
+	}
+	if r.Header.Get("X-Forwarded-For") != "" {
+		if !waf.IsMalformedForwardedFor(r.Header.Get("X-Forwarded-For"), 7, 255) {
+			return parseSourceAddress(r.Header.Get("X-Forwarded-For"))
+		}
+	}
+	return parseSourceAddress(r.RemoteAddr)
+}
+
 // GetSourceConnAddress returns the IP address of the HTTP connection.
 func GetSourceConnAddress(r *http.Request) string {
 	addr := r.RemoteAddr
@@ -106,19 +119,39 @@ func parseAddr6(s string) string {
 
 // GetTargetURL returns the URL the user landed on.
 func GetTargetURL(r *http.Request) string {
+	s, _ := GetCurrentURLWithSuffix(r, "")
+	return s
+}
+
+// GetCurrentURLWithSuffix returns current URL based on the provided suffux.
+func GetCurrentURLWithSuffix(r *http.Request, suffix string) (string, error) {
 	h := r.Header.Get("X-Forwarded-Host")
+
+	if waf.IsMalformedForwardedHost(h, 2, 255) {
+		return malformedURLStr, fmt.Errorf("malformed X-Forwarded-Host header")
+	}
+
 	if h == "" {
 		h = r.Host
 	}
+
 	p := r.Header.Get("X-Forwarded-Proto")
+	if waf.IsMalformedForwardedProto(p, 2, 10) {
+		return malformedURLStr, fmt.Errorf("malformed X-Forwarded-Proto header")
+	}
+
 	if p == "" {
 		if r.TLS == nil {
 			p = "http"
 		} else {
 			p = "https"
 		}
 	}
+
 	port := r.Header.Get("X-Forwarded-Port")
+	if waf.IsMalformedForwardedPort(port, 2, 5) {
+		return malformedURLStr, fmt.Errorf("malformed X-Forwarded-Port header")
+	}
 
 	u := p + "://" + h
 
@@ -136,5 +169,13 @@ func GetTargetURL(r *http.Request) string {
 			u += ":" + port
 		}
 	}
-	return u + r.RequestURI
+	if suffix != "" {
+		i := strings.Index(r.RequestURI, suffix)
+		if i < 0 {
+			return u + r.RequestURI, nil
+		}
+		return u + r.RequestURI[:i] + suffix, nil
+	}
+
+	return u + r.RequestURI, nil
 }