Statsd EscapeTag is incomplete: newline/CR enables wire-protocol injection #14

Summary

EscapeTag strips :, |, ,, and @ from tag values to keep them from confusing the statsd parser. It does not strip \n or \r, which are message delimiters in the statsd wire protocol. Any input that flows through a tag can therefore inject one or more arbitrary statsd messages onto the wire — including counters, gauges, and timers — attributed to the connectivity process.

Code

statsd.go:33-40:

func EscapeTag(s string) string {
	// Replace all special characters used in the statsd wire protocol
	s = strings.Replace(s, ":", "-", -1)
	s = strings.Replace(s, "|", "-", -1)
	s = strings.Replace(s, ",", "-", -1)
	s = strings.Replace(s, "@", "-", -1)
	return s                                   // \n and \r pass through
}

destinations.go:68-76 feeds user-controlled values (Label, Host, Scheme, Protocol) into tags:

func (dest *Destination) tags() []string {
	return []string{
		fmt.Sprintf("dest_label:%s", EscapeTag(dest.Label)),
		fmt.Sprintf("dest_scheme:%s", EscapeTag(dest.Scheme)),
		fmt.Sprintf("dest_host:%s", EscapeTag(dest.Host)),
		...
	}
}

Reproduction

# connectivity.yml — Label contains a newline + a forged metric
"normal\ninjected_metric:9999|c|#owned=true": http://example.com/

When connectivity check fires connectivity.check:1|c|#dest_label:normal\ninjected_metric:9999|c|#owned=true,..., a TCP statsd server (or any server that splits packets on \n, which is typical) parses this as two messages, the second of which is injected_metric:9999|c|#owned=true. The injection appears in metrics dashboards.

The same is true for any field that ends up in a tag — host names in URLs can't contain raw \n after Go's URL parser is done, but YAML keys (Labels) are unrestricted.

Impact

• Operators with multi-tenant statsd back-ends can have metrics polluted by anyone who controls a connectivity config.
• Combined with dashboards/alerts keyed on metric names, an attacker can fabricate alerts ("everything is on fire") or hide real ones ("everything looks fine").
• Limited blast radius — the attacker must control YAML config content — but the existing EscapeTag clearly indicates the intent was full escaping.

Suggested fix

Either reject control characters outright, or replace them:

func EscapeTag(s string) string {
	var b strings.Builder
	for _, r := range s {
		switch r {
		case ':', '|', ',', '@', '\n', '\r', '\t':
			b.WriteByte('-')
		default:
			if unicode.IsControl(r) {
				b.WriteByte('-')
			} else {
				b.WriteRune(r)
			}
		}
	}
	return b.String()
}

Also consider validating the metric name itself in Count/Timer/Gauge for the same characters — they currently come from string literals in this codebase, but the helpers are exported.