Skip to content

Commit 870eeec

Browse files
committed
Monitoring metrics: remove sanitizeString
* The client assigns the values to `textContent` of DOM elements. * A malicious user can only control the queried name, other values are from the app itself. * Debug logs and Prometheus metrics need original value. * The admin need to see the original value. Fix #2885.
1 parent d44b0e3 commit 870eeec

1 file changed

Lines changed: 10 additions & 34 deletions

File tree

dnscrypt-proxy/monitoring_ui.go

Lines changed: 10 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"crypto/subtle"
55
"encoding/json"
66
"fmt"
7-
"html"
87
"net"
98
"net/http"
109
"runtime"
@@ -18,25 +17,6 @@ import (
1817
"github.com/miekg/dns"
1918
)
2019

21-
// sanitizeString - Sanitizes user input to prevent XSS attacks
22-
func sanitizeString(input string) string {
23-
// HTML escape to prevent XSS
24-
escaped := html.EscapeString(input)
25-
// Additional validation for domain names - only allow valid domain characters
26-
if strings.Contains(input, ".") { // Likely a domain name
27-
// Remove any non-domain characters
28-
var result strings.Builder
29-
for _, r := range escaped {
30-
if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') ||
31-
(r >= '0' && r <= '9') || r == '.' || r == '-' || r == '_' {
32-
result.WriteRune(r)
33-
}
34-
}
35-
return result.String()
36-
}
37-
return escaped
38-
}
39-
4020
// MonitoringUIConfig - Configuration for the monitoring UI
4121
type MonitoringUIConfig struct {
4222
Enabled bool `toml:"enabled"`
@@ -348,10 +328,9 @@ func (ui *MonitoringUI) UpdateMetrics(pluginsState PluginsState, msg *dns.Msg, s
348328

349329
// Update top domains - separate lock
350330
if mc.privacyLevel < 2 {
351-
sanitizedDomain := sanitizeString(pluginsState.qName)
352331
mc.domainMutex.Lock()
353-
mc.topDomains[sanitizedDomain]++
354-
dlog.Debugf("Domain %s, count: %d", sanitizedDomain, mc.topDomains[sanitizedDomain])
332+
mc.topDomains[pluginsState.qName]++
333+
dlog.Debugf("Domain %s, count: %d", pluginsState.qName, mc.topDomains[pluginsState.qName])
355334
mc.domainMutex.Unlock()
356335
}
357336

@@ -400,11 +379,11 @@ func (ui *MonitoringUI) UpdateMetrics(pluginsState PluginsState, msg *dns.Msg, s
400379
entry := QueryLogEntry{
401380
Timestamp: now,
402381
ClientIP: clientIP,
403-
Domain: sanitizeString(pluginsState.qName),
404-
Type: sanitizeString(qType),
405-
ResponseCode: sanitizeString(returnCode),
382+
Domain: pluginsState.qName,
383+
Type: qType,
384+
ResponseCode: returnCode,
406385
ResponseTime: responseTime,
407-
Server: sanitizeString(pluginsState.serverName),
386+
Server: pluginsState.serverName,
408387
CacheHit: pluginsState.cacheHit,
409388
}
410389

@@ -515,17 +494,15 @@ func (mc *MetricsCollector) generatePrometheusMetrics() string {
515494
result.WriteString("# HELP dnscrypt_proxy_server_queries_total Total queries per server\n")
516495
result.WriteString("# TYPE dnscrypt_proxy_server_queries_total counter\n")
517496
for server, count := range mc.serverQueryCount {
518-
sanitizedServer := sanitizeString(server)
519-
result.WriteString(fmt.Sprintf("dnscrypt_proxy_server_queries_total{server=\"%s\"} %d\n", sanitizedServer, count))
497+
result.WriteString(fmt.Sprintf("dnscrypt_proxy_server_queries_total{server=\"%s\"} %d\n", server, count))
520498
}
521499

522500
result.WriteString("# HELP dnscrypt_proxy_server_response_time_average_ms Average response time per server in milliseconds\n")
523501
result.WriteString("# TYPE dnscrypt_proxy_server_response_time_average_ms gauge\n")
524502
for server, count := range mc.serverQueryCount {
525503
if count > 0 {
526504
avgTime := float64(mc.serverResponseTime[server]) / float64(count)
527-
sanitizedServer := sanitizeString(server)
528-
result.WriteString(fmt.Sprintf("dnscrypt_proxy_server_response_time_average_ms{server=\"%s\"} %.2f\n", sanitizedServer, avgTime))
505+
result.WriteString(fmt.Sprintf("dnscrypt_proxy_server_response_time_average_ms{server=\"%s\"} %.2f\n", server, avgTime))
529506
}
530507
}
531508
mc.serverMutex.RUnlock()
@@ -535,8 +512,7 @@ func (mc *MetricsCollector) generatePrometheusMetrics() string {
535512
result.WriteString("# HELP dnscrypt_proxy_query_type_total Total queries per DNS record type\n")
536513
result.WriteString("# TYPE dnscrypt_proxy_query_type_total counter\n")
537514
for qtype, count := range mc.queryTypes {
538-
sanitizedQtype := sanitizeString(qtype)
539-
result.WriteString(fmt.Sprintf("dnscrypt_proxy_query_type_total{type=\"%s\"} %d\n", sanitizedQtype, count))
515+
result.WriteString(fmt.Sprintf("dnscrypt_proxy_query_type_total{type=\"%s\"} %d\n", qtype, count))
540516
}
541517
mc.queryTypesMutex.RUnlock()
542518

@@ -676,7 +652,7 @@ func (mc *MetricsCollector) GetMetrics() map[string]interface{} {
676652
count := 0
677653
for _, dc := range domainCounts {
678654
topDomainsList = append(topDomainsList, map[string]interface{}{
679-
"domain": sanitizeString(dc.domain),
655+
"domain": dc.domain,
680656
"count": dc.count,
681657
})
682658
count++

0 commit comments

Comments
 (0)