UI/Prom: Remove sanitizeString, and use proper escapes where needed

Fixes #2885
Fixes #2886

Author: jedisct1

dnscrypt-proxy/monitoring_ui.go

@@ -18,25 +18,6 @@ import (
 	"github.com/miekg/dns"
 )
 
-// sanitizeString - Sanitizes user input to prevent XSS attacks
-func sanitizeString(input string) string {
-	// HTML escape to prevent XSS
-	escaped := html.EscapeString(input)
-	// Additional validation for domain names - only allow valid domain characters
-	if strings.Contains(input, ".") { // Likely a domain name
-		// Remove any non-domain characters
-		var result strings.Builder
-		for _, r := range escaped {
-			if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') ||
-				(r >= '0' && r <= '9') || r == '.' || r == '-' || r == '_' {
-				result.WriteRune(r)
-			}
-		}
-		return result.String()
-	}
-	return escaped
-}
-
 // MonitoringUIConfig - Configuration for the monitoring UI
 type MonitoringUIConfig struct {
 	Enabled            bool   `toml:"enabled"`
@@ -360,10 +341,11 @@ func (ui *MonitoringUI) UpdateMetrics(pluginsState PluginsState, msg *dns.Msg, s
 
 	// Update top domains - separate lock
 	if mc.privacyLevel < 2 {
-		sanitizedDomain := sanitizeString(pluginsState.qName)
+		// Store domain name directly - no sanitization needed for internal metrics
+		domainName := pluginsState.qName
 		mc.domainMutex.Lock()
-		mc.topDomains[sanitizedDomain]++
-		dlog.Debugf("Domain %s, count: %d", sanitizedDomain, mc.topDomains[sanitizedDomain])
+		mc.topDomains[domainName]++
+		dlog.Debugf("Domain %s, count: %d", domainName, mc.topDomains[domainName])
 		mc.domainMutex.Unlock()
 	}
 
@@ -410,13 +392,14 @@ func (ui *MonitoringUI) UpdateMetrics(pluginsState PluginsState, msg *dns.Msg, s
 		}
 
 		entry := QueryLogEntry{
-			Timestamp:    now,
-			ClientIP:     clientIP,
-			Domain:       sanitizeString(pluginsState.qName),
-			Type:         sanitizeString(qType),
-			ResponseCode: sanitizeString(returnCode),
+			Timestamp: now,
+			ClientIP:  clientIP,
+			// HTML escape only the fields that will be displayed in web UI
+			Domain:       html.EscapeString(pluginsState.qName),
+			Type:         qType,      // DNS types are safe, no escaping needed
+			ResponseCode: returnCode, // DNS response codes are safe, no escaping needed
 			ResponseTime: responseTime,
-			Server:       sanitizeString(pluginsState.serverName),
+			Server:       html.EscapeString(pluginsState.serverName),
 			CacheHit:     pluginsState.cacheHit,
 		}
 
@@ -527,17 +510,19 @@ func (mc *MetricsCollector) generatePrometheusMetrics() string {
 	result.WriteString("# HELP dnscrypt_proxy_server_queries_total Total queries per server\n")
 	result.WriteString("# TYPE dnscrypt_proxy_server_queries_total counter\n")
 	for server, count := range mc.serverQueryCount {
-		sanitizedServer := sanitizeString(server)
-		result.WriteString(fmt.Sprintf("dnscrypt_proxy_server_queries_total{server=\"%s\"} %d\n", sanitizedServer, count))
+		// For Prometheus labels, escape quotes and backslashes to prevent label injection
+		escapedServer := strings.ReplaceAll(strings.ReplaceAll(server, "\\", "\\\\"), "\"", "\\\"")
+		result.WriteString(fmt.Sprintf("dnscrypt_proxy_server_queries_total{server=\"%s\"} %d\n", escapedServer, count))
 	}
 
 	result.WriteString("# HELP dnscrypt_proxy_server_response_time_average_ms Average response time per server in milliseconds\n")
 	result.WriteString("# TYPE dnscrypt_proxy_server_response_time_average_ms gauge\n")
 	for server, count := range mc.serverQueryCount {
 		if count > 0 {
 			avgTime := float64(mc.serverResponseTime[server]) / float64(count)
-			sanitizedServer := sanitizeString(server)
-			result.WriteString(fmt.Sprintf("dnscrypt_proxy_server_response_time_average_ms{server=\"%s\"} %.2f\n", sanitizedServer, avgTime))
+			// For Prometheus labels, escape quotes and backslashes to prevent label injection
+			escapedServer := strings.ReplaceAll(strings.ReplaceAll(server, "\\", "\\\\"), "\"", "\\\"")
+			result.WriteString(fmt.Sprintf("dnscrypt_proxy_server_response_time_average_ms{server=\"%s\"} %.2f\n", escapedServer, avgTime))
 		}
 	}
 	mc.serverMutex.RUnlock()
@@ -547,8 +532,8 @@ func (mc *MetricsCollector) generatePrometheusMetrics() string {
 	result.WriteString("# HELP dnscrypt_proxy_query_type_total Total queries per DNS record type\n")
 	result.WriteString("# TYPE dnscrypt_proxy_query_type_total counter\n")
 	for qtype, count := range mc.queryTypes {
-		sanitizedQtype := sanitizeString(qtype)
-		result.WriteString(fmt.Sprintf("dnscrypt_proxy_query_type_total{type=\"%s\"} %d\n", sanitizedQtype, count))
+		// DNS query types are safe alphanumeric values, no escaping needed
+		result.WriteString(fmt.Sprintf("dnscrypt_proxy_query_type_total{type=\"%s\"} %d\n", qtype, count))
 	}
 	mc.queryTypesMutex.RUnlock()
 
@@ -688,7 +673,7 @@ func (mc *MetricsCollector) GetMetrics() map[string]interface{} {
 		count := 0
 		for _, dc := range domainCounts {
 			topDomainsList = append(topDomainsList, map[string]interface{}{
-				"domain": sanitizeString(dc.domain),
+				"domain": html.EscapeString(dc.domain),
 				"count":  dc.count,
 			})
 			count++