From d9f8d7fbc53dfef25f8a8b260274afcade86ed42 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 18 May 2016 18:58:04 +0000 Subject: [PATCH 1/1] 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. --- lib/PublicInbox/NNTP.pm | 4 ++-- t/nntpd.t | 30 +++++++++++++++++++++++------- 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; -- 2.44.0