Changeset 766

Timestamp:

03/09/08 03:58:16 (2 years ago)

Author:

bradfitz

Message:

SECURITY: patch from Jeremey James <jbj@…> to not crash
on zero byte chunked upload when buffered uploads are enabled.

Location:

trunk

Files:

• CHANGES (modified) (2 diffs)
• lib/Perlbal/ClientProxy.pm (modified) (3 diffs)
• t/52-chunked-upload.t (modified) (1 diff)

trunk/CHANGES

@@ +1 @@
+    -- SECURITY: patch from Jeremey James <jbj@forbidden.co.uk> to not crash
+       on zero byte chunked upload when buffered uploads are enabled.
+
     -- on successful write, update Perlbal::Socket's alive_time, so slowly
        reproxied writes don't timeout the connection and kill it.  Patch
@@ -12 +15 @@
        to bring in more config; can be nested.
 
-    -- SECURITY - Previously a single upward directory traversal was possible
+    -- SECURITY: Previously a single upward directory traversal was possible
        when concat get was enabled. This behavior has been fixed in code to
        match with standard file serving.

trunk/lib/Perlbal/ClientProxy.pm

@@ -1021 +1021 @@
     # reset our position so we start reading from the right spot
     $self->{buoutpos} = 0;
-    sysseek($self->{bufh}, 0, 0);
+    sysseek($self->{bufh}, 0, 0) if ($self->{bufh}); # But only if it exists at all
 
     # notify that we want the backend so we get the ball rolling
@@ -1035 +1035 @@
     my $clen = $self->{request_body_length};
 
-    my $sent = Perlbal::Socket::sendfile($be->{fd}, fileno($self->{bufh}), $clen - $self->{buoutpos});
-    if ($sent < 0) {
-        return $self->close("epipe") if $! == EPIPE;
-        return $self->close("connreset") if $! == ECONNRESET;
-        print STDERR "Error w/ sendfile: $!\n";
-        return $self->close('sendfile_error');
-    }
-    $self->{buoutpos} += $sent;
+    if ($self->{buoutpos} < $clen) {
+        my $sent = Perlbal::Socket::sendfile($be->{fd}, fileno($self->{bufh}), $clen - $self->{buoutpos});
+        if ($sent < 0) {
+            return $self->close("epipe") if $! == EPIPE;
+            return $self->close("connreset") if $! == ECONNRESET;
+            print STDERR "Error w/ sendfile: $!\n";
+            return $self->close('sendfile_error');
+        }
+        $self->{buoutpos} += $sent;
+    }
 
     # if we're done, purge the file and move on
@@ -1155 +1157 @@
 sub purge_buffered_upload {
     my Perlbal::ClientProxy $self = shift;
+
+    # Main reason for failure below is a 0-length chunked upload, where the file is never created.
+    return unless $self->{bufh};
 
     # FIXME: it's reported that sometimes the two now-in-eval blocks

trunk/t/52-chunked-upload.t

@@ -184 +184 @@
 }
 
+# Try a 0 length chunked request, as it used to crash server
+{
+    my $hdr = "POST /status HTTP/1.0\r\nTransfer-Encoding: chunked\r\n\r\n0\r\n\r\n";
+    my $sock = IO::Socket::INET->new( PeerAddr => "127.0.0.1:$port" )
+        or return undef;
+    my $rv = syswrite($sock, $hdr);
+    die unless $rv == length($hdr);
+
+    # Give it time to crash
+    select undef, undef, undef, 1.0;
+
+    my $sock2 = IO::Socket::INET->new( PeerAddr => "127.0.0.1:$port" );
+    ok ($sock2, 'Server still alive');
+}
+
 1;