fix(scan): baseline-diff sql errors against shared templates (#304)

checkDatabaseErrors matched db error regexes against each response with no baseline, so a site serving a shared error template, custom 500 or waf block page containing a db error string reported an identical high-severity sql injection finding on every probed path. capture the main-url response as a baseline and skip, per pattern, any error string already present in it while still checking other patterns, so only differential probe-caused errors are reported.
2026-07-28 14:37:01 -07:00 · 2026-07-22 12:48:31 -07:00
parent 4f01345974
commit 9fce39cd32
2 changed files with 158 additions and 32 deletions
@@ -230,8 +230,10 @@ func SQL(targetURL string, timeout time.Duration, threads int, logdir string, ca
 	}
 	wg.Wait()

-	// check main URL for database errors
-	checkDatabaseErrors(client, targetURL, sanitizedURL, result, logdir, &mu, seenErrors)
+	// check main URL for database errors; its body doubles as the baseline
+	// for every probe below, since a shared error template or WAF block page
+	// can echo the same error string for any path regardless of injection.
+	baselineBody := checkDatabaseErrors(client, targetURL, sanitizedURL, result, logdir, &mu, seenErrors, "")

 	// check common endpoints that might expose database errors
 	errorCheckPaths := []string{
@@ -246,7 +248,7 @@ func SQL(targetURL string, timeout time.Duration, threads int, logdir string, ca

 	for _, path := range errorCheckPaths {
 		checkURL := strings.TrimSuffix(targetURL, "/") + path
-		checkDatabaseErrors(client, checkURL, sanitizedURL, result, logdir, &mu, seenErrors)
+		checkDatabaseErrors(client, checkURL, sanitizedURL, result, logdir, &mu, seenErrors, baselineBody)
 	}

 	spin.Stop()
@@ -332,49 +334,63 @@ func isAdminPanel(body string, panelType string) bool {
 	}
 }

-func checkDatabaseErrors(client *http.Client, checkURL, sanitizedURL string, result *SQLResult, logdir string, mu *sync.Mutex, seen map[string]bool) {
+// checkDatabaseErrors probes checkURL for database error signatures and returns
+// the body so callers can reuse it as a baseline. baseline is an earlier
+// request's body (empty on the first call); a pattern also present in
+// baseline is skipped, since it is not attributable to this probe.
+func checkDatabaseErrors(client *http.Client, checkURL, sanitizedURL string, result *SQLResult, logdir string, mu *sync.Mutex, seen map[string]bool, baseline string) string {
 	req, err := http.NewRequestWithContext(context.TODO(), http.MethodGet, checkURL, http.NoBody)
 	if err != nil {
-		return
+		return ""
 	}
 	resp, err := client.Do(req)
 	if err != nil {
-		return
+		return ""
 	}
 	defer resp.Body.Close()

 	body, err := io.ReadAll(io.LimitReader(resp.Body, 1024*100))
 	if err != nil {
-		return
+		return ""
 	}
 	bodyStr := string(body)

 	for _, pattern := range databaseErrorPatterns {
-		if pattern.pattern.MatchString(bodyStr) {
-			key := checkURL + "|" + pattern.databaseType
-			mu.Lock()
-			if seen[key] {
-				mu.Unlock()
-				break
-			}
-			seen[key] = true
-
-			dbError := SQLDatabaseError{
-				URL:          checkURL,
-				DatabaseType: pattern.databaseType,
-				ErrorPattern: pattern.pattern.String(),
-			}
-			result.DatabaseErrors = append(result.DatabaseErrors, dbError)
-			mu.Unlock()
-
-			output.Warn("Database error disclosure: %s at %s",
-				output.SeverityHigh.Render(pattern.databaseType),
-				output.Highlight.Render(checkURL))
-
-			if logdir != "" {
-				logger.Write(sanitizedURL, logdir, "Database error disclosure: "+pattern.databaseType+" at ["+checkURL+"]\n")
-			}
-			break // only report one database type per URL
+		if !pattern.pattern.MatchString(bodyStr) {
+			continue
 		}
+		if baseline != "" && pattern.pattern.MatchString(baseline) {
+			// same error text is already present on the unmodified page,
+			// so this probe did not cause it; keep looking for a pattern
+			// that is actually differential.
+			continue
+		}
+
+		key := checkURL + "|" + pattern.databaseType
+		mu.Lock()
+		if seen[key] {
+			mu.Unlock()
+			break
+		}
+		seen[key] = true
+
+		dbError := SQLDatabaseError{
+			URL:          checkURL,
+			DatabaseType: pattern.databaseType,
+			ErrorPattern: pattern.pattern.String(),
+		}
+		result.DatabaseErrors = append(result.DatabaseErrors, dbError)
+		mu.Unlock()
+
+		output.Warn("Database error disclosure: %s at %s",
+			output.SeverityHigh.Render(pattern.databaseType),
+			output.Highlight.Render(checkURL))
+
+		if logdir != "" {
+			logger.Write(sanitizedURL, logdir, "Database error disclosure: "+pattern.databaseType+" at ["+checkURL+"]\n")
+		}
+		break // only report one database type per URL
 	}
+
+	return bodyStr
 }
@@ -15,6 +15,7 @@ package scan
 import (
 	"net/http"
 	"net/http/httptest"
+	"sync"
 	"testing"
 )

@@ -278,3 +279,112 @@ func TestSQLDatabaseErrorDetection(t *testing.T) {
 		t.Errorf("expected status 200, got %d", resp.StatusCode)
 	}
 }
+
+// TestCheckDatabaseErrors_SharedTemplateNotFlagged proves a db error string
+// present on every response, including non-injection paths, is not reported.
+func TestCheckDatabaseErrors_SharedTemplateNotFlagged(t *testing.T) {
+	const sharedTemplate = "<html>Debug: mysql_fetch_array() failed on this handler</html>"
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(sharedTemplate))
+	}))
+	defer server.Close()
+
+	client := server.Client()
+	result := &SQLResult{}
+	seen := make(map[string]bool)
+	var mu sync.Mutex
+
+	// fetched directly rather than through checkDatabaseErrors: this test only
+	// covers suppression of later probes, not the homepage's own content.
+	resp, err := client.Get(server.URL + "/")
+	if err != nil {
+		t.Fatalf("failed to fetch baseline: %v", err)
+	}
+	defer resp.Body.Close()
+	baselineBody := sharedTemplate
+
+	checkDatabaseErrors(client, server.URL+"/login", server.URL, result, "", &mu, seen, baselineBody)
+	checkDatabaseErrors(client, server.URL+"/?id=1'", server.URL, result, "", &mu, seen, baselineBody)
+
+	if len(result.DatabaseErrors) != 0 {
+		t.Errorf("expected no differential database errors, got %d: %+v", len(result.DatabaseErrors), result.DatabaseErrors)
+	}
+}
+
+// TestCheckDatabaseErrors_DifferentialErrorFlagged proves the fix does not
+// suppress a real, injection-caused disclosure: only the probed path shows
+// the error string, the baseline page does not.
+func TestCheckDatabaseErrors_DifferentialErrorFlagged(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		if r.URL.RawQuery == "id=1'" {
+			w.Write([]byte("MySQL Error: You have an error in your SQL syntax"))
+			return
+		}
+		w.Write([]byte("Welcome to our website"))
+	}))
+	defer server.Close()
+
+	client := server.Client()
+	result := &SQLResult{}
+	seen := make(map[string]bool)
+	var mu sync.Mutex
+
+	baseline := checkDatabaseErrors(client, server.URL+"/", server.URL, result, "", &mu, seen, "")
+	checkDatabaseErrors(client, server.URL+"/?id=1'", server.URL, result, "", &mu, seen, baseline)
+
+	if len(result.DatabaseErrors) != 1 {
+		t.Fatalf("expected 1 differential database error, got %d: %+v", len(result.DatabaseErrors), result.DatabaseErrors)
+	}
+	if result.DatabaseErrors[0].DatabaseType != "MySQL" {
+		t.Errorf("expected database type 'MySQL', got '%s'", result.DatabaseErrors[0].DatabaseType)
+	}
+}
+
+// TestCheckDatabaseErrors_PerPatternDifferential proves the baseline skip is
+// per-pattern, not global: a differential postgresql leak must still be
+// reported even though the response also carries a shared, baseline mysql string.
+func TestCheckDatabaseErrors_PerPatternDifferential(t *testing.T) {
+	const sharedMySQL = "<!-- rendered by mysql_fetch_array helper -->"
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		if r.URL.RawQuery == "id=1'" {
+			_, _ = w.Write([]byte(sharedMySQL + "\npg_query() failed: ERROR: unterminated quoted string"))
+			return
+		}
+		_, _ = w.Write([]byte(sharedMySQL + "\nWelcome"))
+	}))
+	defer server.Close()
+
+	client := server.Client()
+	result := &SQLResult{}
+	seen := make(map[string]bool)
+	var mu sync.Mutex
+
+	baseline := checkDatabaseErrors(client, server.URL+"/", server.URL, result, "", &mu, seen, "")
+	// homepage has no baseline of its own, so its mysql marker is reported once.
+	if len(result.DatabaseErrors) != 1 || result.DatabaseErrors[0].DatabaseType != "MySQL" {
+		t.Fatalf("expected homepage to report its own mysql marker once, got %+v", result.DatabaseErrors)
+	}
+
+	checkDatabaseErrors(client, server.URL+"/?id=1'", server.URL, result, "", &mu, seen, baseline)
+
+	var gotPostgres, gotProbedMySQL bool
+	for _, e := range result.DatabaseErrors {
+		if e.URL == server.URL+"/?id=1'" && e.DatabaseType == "PostgreSQL" {
+			gotPostgres = true
+		}
+		if e.URL == server.URL+"/?id=1'" && e.DatabaseType == "MySQL" {
+			gotProbedMySQL = true
+		}
+	}
+	if !gotPostgres {
+		t.Errorf("per-pattern differential failed: postgresql leak on the probed path was suppressed (critical false negative): %+v", result.DatabaseErrors)
+	}
+	if gotProbedMySQL {
+		t.Errorf("shared mysql pattern should have been suppressed on the probed path, but was reported: %+v", result.DatabaseErrors)
+	}
+}