diff --git a/internal/exporter/export.go b/internal/exporter/export.go index 1825974fd..d8069ad60 100644 --- a/internal/exporter/export.go +++ b/internal/exporter/export.go @@ -17,6 +17,7 @@ import ( "strings" "sync" "time" + "unicode" "github.com/golang/glog" "github.com/google/mtail/internal/metrics" @@ -154,6 +155,32 @@ func (e *Exporter) SetOption(options ...Option) error { return nil } +// sanitizeLabel replaces whitespace and control characters in a label key or +// value with rep. Label values are derived from log line contents (for example +// an HTTP User-Agent captured with [[:print:]]+, or getfilename()), so a space +// or newline in that data would otherwise split a graphite/statsd/collectd line +// into extra fields or a whole extra record, letting a crafted log line spoof or +// inject metrics on the wire. The separator replacement above only covers ksep +// and sep, so these characters have to be handled here as well. +func sanitizeLabel(s, rep string) string { + if strings.IndexFunc(s, isUnsafeLabelRune) < 0 { + return s + } + var b strings.Builder + for _, r := range s { + if isUnsafeLabelRune(r) { + b.WriteString(rep) + continue + } + b.WriteRune(r) + } + return b.String() +} + +func isUnsafeLabelRune(r rune) bool { + return unicode.IsControl(r) || unicode.IsSpace(r) +} + // formatLabels converts a metric name and key-value map of labels to a single // string for exporting to the correct output format for each export target. // ksep and sep mark what to use for key/val separator, and between label separators respoectively. @@ -168,8 +195,8 @@ func formatLabels(name string, m map[string]string, ksep, sep, rep string) strin sort.Strings(keys) var s []string for _, k := range keys { - k1 := strings.ReplaceAll(strings.ReplaceAll(k, ksep, rep), sep, rep) - v1 := strings.ReplaceAll(strings.ReplaceAll(m[k], ksep, rep), sep, rep) + k1 := sanitizeLabel(strings.ReplaceAll(strings.ReplaceAll(k, ksep, rep), sep, rep), rep) + v1 := sanitizeLabel(strings.ReplaceAll(strings.ReplaceAll(m[k], ksep, rep), sep, rep), rep) s = append(s, fmt.Sprintf("%s%s%s", k1, ksep, v1)) } return r + sep + strings.Join(s, sep) diff --git a/internal/exporter/export_test.go b/internal/exporter/export_test.go index 317cbba06..a3d7d9cee 100644 --- a/internal/exporter/export_test.go +++ b/internal/exporter/export_test.go @@ -160,6 +160,33 @@ func TestMetricToGraphite(t *testing.T) { testutil.ExpectNoDiff(t, expected, r) } +func TestFormatLabelsSanitizesWhitespace(t *testing.T) { + *graphitePrefix = "" + ts, terr := time.Parse("2006/01/02 15:04:05", "2012/07/24 10:14:00") + if terr != nil { + t.Errorf("time parse error: %s", terr) + } + + // A label value derived from a log line (e.g. an HTTP User-Agent) that + // carries a space and an embedded newline must not split the graphite line + // into extra fields or an injected second record. + m := metrics.NewMetric("bar", "prog", metrics.Gauge, metrics.Int, "ua") + d, _ := m.GetDatum("Mozilla 5.0\ninjected.metric 999 1") + datum.SetInt(d, 37, ts) + r := FakeSocketWrite(metricToGraphite, m) + expected := []string{"prog.bar.ua.Mozilla_5_0_injected_metric_999_1 37 1343124840\n"} + testutil.ExpectNoDiff(t, expected, r) + for _, line := range r { + if strings.Count(line, "\n") != 1 { + t.Errorf("graphite line contains injected record: %q", line) + } + path := strings.SplitN(line, " ", 2)[0] + if strings.ContainsAny(path, " \t\r\n") { + t.Errorf("graphite metric path contains whitespace: %q", line) + } + } +} + func TestMetricToStatsd(t *testing.T) { *statsdPrefix = "" ts, terr := time.Parse("2006/01/02 15:04:05", "2012/07/24 10:14:00")