nntpd: reject control characters entirely

There's no place for them in the commands and we don't take
messages; potentially printing them into a log opened in a
terminal is too dangerous.

Hoist out read_til_dot in the test while we're at it.

2 files changed, 25 insertions, 9 deletions

diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index e77ccaa4..ac536f71 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -12,7 +12,6 @@ use PublicInbox::Msgmap;
 use PublicInbox::Git;
 use PublicInbox::MID qw(mid2path);
 use Email::Simple;
-use Data::Dumper qw(Dumper);
 use POSIX qw(strftime);
 use Time::HiRes qw(clock_gettime CLOCK_MONOTONIC);
 use URI::Escape qw(uri_escape_utf8);
@@ -151,7 +150,7 @@ sub process_line ($$) {
         my $res = eval { $req->($self, @args) };
         my $err = $@;
         if ($err && !$self->{closed}) {
-                chomp($l = Dumper(\$l));
+                chomp($l);
                 err($self, 'error from: %s (%s)', $l, $err);
                 $res = '503 program fault - command not performed';
         }
@@ -972,6 +971,7 @@ sub event_read {
         $self->{rbuf} .= $$buf;
         while ($r > 0 && $self->{rbuf} =~ s/\A\s*([^\r\n]+)\r?\n//) {
                 my $line = $1;
+                return $self->close if $line =~ /[[:cntrl:]]/s;
                 my $t0 = now();
                 my $fd = $self->{fd};
                 $r = eval { process_line($self, $line) };
diff --git a/t/nntpd.t b/t/nntpd.t
index 60cf8938..837b9d46 100644
--- a/t/nntpd.t
+++ b/t/nntpd.t
@@ -118,6 +118,18 @@ EOF
         is($buf, "201 server ready - post via email\r\n", 'got greeting');
         $s->autoflush(1);
 
+        syswrite($s, "NEWGROUPS\t19990424 000000 \033GMT\007\r\n");
+        is(0, sysread($s, $buf, 4096), 'GOT EOF on cntrl');
+
+        $s = IO::Socket::INET->new(%opts);
+        sysread($s, $buf, 4096);
+        is($buf, "201 server ready - post via email\r\n", 'got greeting');
+        $s->autoflush(1);
+
+        syswrite($s, "NEWGROUPS 19990424 000000 GMT\r\n");
+        $buf = read_til_dot($s);
+        like($buf, qr/\A231 list of /, 'newgroups OK');
+
         while (my ($k, $v) = each %xhdr) {
                 is_deeply($n->xhdr("$k $mid"), { $mid => $v },
                           "XHDR $k by message-id works");
@@ -127,9 +139,7 @@ EOF
                           "$k by article range works");
                 $buf = '';
                 syswrite($s, "HDR $k $mid\r\n");
-                do {
-                        sysread($s, $buf, 4096, length($buf));
-                } until ($buf =~ /\r\n\.\r\n\z/);
+                $buf = read_til_dot($s);
                 my @r = split("\r\n", $buf);
                 like($r[0], qr/\A225 /, '225 response for HDR');
                 is($r[1], "0 $v", 'got expected response for HDR');
@@ -163,10 +173,7 @@ EOF
 
         {
                 syswrite($s, "OVER $mid\r\n");
-                $buf = '';
-                do {
-                        sysread($s, $buf, 4096, length($buf));
-                } until ($buf =~ /\r\n\.\r\n\z/);
+                $buf = read_til_dot($s);
                 my @r = split("\r\n", $buf);
                 like($r[0], qr/^224 /, 'got 224 response for OVER');
                 is($r[1], "0\tTesting for El\xc3\xa9anor\t" .
@@ -212,4 +219,13 @@ EOF
 
 done_testing();
 
+sub read_til_dot {
+        my ($s) = @_;
+        my $buf = '';
+        do {
+                sysread($s, $buf, 4096, length($buf));
+        } until ($buf =~ /\r\n\.\r\n\z/);
+        $buf;
+}
+
 1;