From: CPANSec Security Scanner Bot <cpan-security@security.metacpan.org>
Subject: [PATCH] Net::Statsite::Client: validate/neutralise metric name and value (CWE-93)

The send() method serialises each metric onto the statsd/statsite UDP
wire format as "$key:$value\n", where ':' and '|' are field separators
and '\n' is the record SEPARATOR. The existing key sanitiser only
strips ':', '|' and '/' (Client.pm:179, missing the newline) and the
value is not sanitised at all, so a metric name OR value derived from
user input can both forge additional metric lines (cross-record, via
'\n') and corrupt/redirect the current record (within-record, via an
injected '|' that adds a phantom type or sample-rate field):

    $s->increment("login.$user");   # $user = "x\nattacker:1|c"    (name, cross-record)
    $s->gauge("size", $len);        # $len  = "1\nattacker:0|c"    (value, cross-record)
    $s->unique("ip.unique", $ip);   # $ip   = "1.2.3.4\nattacker:9|c"
    $s->gauge("reqs", $n);          # $n    = "1|c|\@0.0001"        (value, within-record)

All public methods (increment/decrement/update/timing/gauge/unique)
funnel through send(), so this is the default path.

Fix, two parts sharing one separator+control character class
([:|\/\x00-\x1f\x7f] — statsite separators plus C0 control bytes and
DEL, which includes \n and \r):

  1. Key (send()): the class is applied to the metric name, extending
     the existing strip to cover the record-separating newline.

  2. Value (_clean_value(), called from timing/update/unique/gauge):
     applied to each user-supplied value BEFORE its structural
     "|<type>" suffix is appended — because once the value reaches
     send() that suffix is already part of the string and stripping
     '|' there would destroy every metric's type marker. timing/update/
     gauge values are numeric by contract (integer or decimal, possibly
     negative), and a plain number contains none of the dangerous
     characters, so _clean_value() accepts a valid number unchanged and
     only rewrites anything else. unique() items are arbitrary (e.g. an
     IP or string), so they take the strip path. The numeric pattern
     /^-?(?:0|[1-9][0-9]*)(?:\.[0-9]+)?\z/ accepts integers, zero and
     negatives (so increment's 1 / decrement's -1 / a gauge of 0 stay
     valid) with an optional decimal fraction.

--- a/lib/Net/Statsite/Client.pm
+++ b/lib/Net/Statsite/Client.pm
@@ -71,6 +71,7 @@

 sub timing {
     my ($self, $stat, $time, $sample_rate) = @_;
+    $time = _clean_value($time);
     $self->send({ $stat => "$time|ms" }, $sample_rate);
 }

@@ -105,6 +106,7 @@
 sub update {
     my ($self, $stats, $delta, $sample_rate) = @_;
     $delta = 1 unless defined $delta;
+    $delta = _clean_value($delta);
     my %data;
     if (ref($stats) eq 'ARRAY') {
         %data = map { $_ => "$delta|c" } @$stats;
@@ -126,6 +128,7 @@

 sub unique {
     my ($self, $stats, $item, $sample_rate) = @_;
+    $item = _clean_value($item);
     my %data = ($stats => "$item|s");
     $self->send(\%data, $sample_rate);
 }
@@ -138,6 +141,7 @@

 sub gauge {
     my ($self, $stats, $value, $sample_rate) = @_;
+    $value = _clean_value($value);
     my %data = ($stats => "$value|g");
     $self->send(\%data, $sample_rate);
 }
@@ -174,9 +178,14 @@
             $key = "$$self{ prefix }.$stat";
         }

-        #sanitize key (remove statsite separators)
+        #sanitize key: remove statsite separators (: | /) and control
+        #bytes (\n \r etc.) in one class. The newline is the metric
+        #record separator, so a user-derived name containing one would
+        #otherwise forge additional metric lines. User-supplied VALUES
+        #are sanitised separately in timing/update/unique/gauge via
+        #_clean_value(), before their structural "|<type>" suffix.
         #https://github.com/armon/statsite#protocol
-        $key =~ s/[:|\/]/_/g;
+        $key =~ s/[:|\/\x00-\x1f\x7f]/_/g;

         _send_to_sock($socket, "$key:$value\n");
         ++$count;
@@ -184,6 +193,22 @@
     return $count;
 }

+#Sanitise a user-supplied metric VALUE before its "|<type>" suffix is
+#appended. timing/update/gauge values are numeric (integer or decimal,
+#optionally negative); a plain number cannot carry a statsite separator
+#or a control byte, so it is accepted unchanged. Anything else — a
+#unique() item such as an IP/string, or hostile input containing
+#newlines / ':' / '|' / '/' — gets the same separator + control-byte
+#strip as the key, so it can neither forge a new record nor inject an
+#extra '|'-delimited field into the current one (CWE-93).
+sub _clean_value {
+    my $v = shift;
+    return $v unless defined $v;
+    return $v if $v =~ /^-?(?:0|[1-9][0-9]*)(?:\.[0-9]+)?\z/;
+    $v =~ s/[:|\/\x00-\x1f\x7f]/_/g;
+    return $v;
+}
+
 sub _send_to_sock( $$ ) {
     my ($sock, $msg) = @_;
     CORE::send($sock, $msg, 0);
