diff --git a/internal/scan/sql.go b/internal/scan/sql.go index 36fb570..3ad41f2 100644 --- a/internal/scan/sql.go +++ b/internal/scan/sql.go @@ -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 } diff --git a/internal/scan/sql_test.go b/internal/scan/sql_test.go index 3c77e41..e8992e6 100644 --- a/internal/scan/sql_test.go +++ b/internal/scan/sql_test.go @@ -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 = "Debug: mysql_fetch_array() failed on this handler" + + 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 = "" + + 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) + } +}