|
From f95113402b7239f225282806673e1b6424522b18 Mon Sep 17 00:00:00 2001
|
|
From: Eric Wong <normalperson@yhbt.net>
|
|
Date: Wed, 22 Aug 2012 22:48:23 +0000
|
|
Subject: [PATCH] multipart/parser: avoid unbounded #gets method
|
|
|
|
Malicious clients may send excessively long lines
|
|
to trigger out-of-memory errors in a Rack web server.
|
|
---
|
|
lib/rack/multipart/parser.rb | 13 ++++++++---
|
|
test/spec_multipart.rb | 53 ++++++++++++++++++++++++++++++++++++++++++
|
|
2 files changed, 63 insertions(+), 3 deletions(-)
|
|
|
|
diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb
|
|
index 21986cc..1315c7b 100644
|
|
--- a/lib/rack/multipart/parser.rb
|
|
+++ b/lib/rack/multipart/parser.rb
|
|
@@ -70,9 +70,16 @@ def rx
|
|
|
|
def fast_forward_to_first_boundary
|
|
loop do
|
|
- read_buffer = @io.gets
|
|
- break if read_buffer == full_boundary
|
|
- raise EOFError, "bad content body" if read_buffer.nil?
|
|
+ content = @io.read(BUFSIZE)
|
|
+ raise EOFError, "bad content body" unless content
|
|
+ @buf << content
|
|
+
|
|
+ while @buf.gsub!(/\A([^\n]*\n)/, '')
|
|
+ read_buffer = $1
|
|
+ return if read_buffer == full_boundary
|
|
+ end
|
|
+
|
|
+ raise EOFError, "bad content body" if Utils.bytesize(@buf) >= BUFSIZE
|
|
end
|
|
end
|
|
|
|
diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb
|
|
index 88729ee..ca3f974 100644
|
|
--- a/test/spec_multipart.rb
|
|
+++ b/test/spec_multipart.rb
|
|
@@ -48,6 +48,59 @@ def multipart_file(name)
|
|
params['profile']['bio'].should.include 'hello'
|
|
end
|
|
|
|
+ should "reject insanely long boundaries" do
|
|
+ # using a pipe since a tempfile can use up too much space
|
|
+ rd, wr = IO.pipe
|
|
+
|
|
+ # we only call rewind once at start, so make sure it succeeds
|
|
+ # and doesn't hit ESPIPE
|
|
+ def rd.rewind; end
|
|
+ wr.sync = true
|
|
+
|
|
+ # mock out length to make this pipe look like a Tempfile
|
|
+ def rd.length
|
|
+ 1024 * 1024 * 8
|
|
+ end
|
|
+
|
|
+ # write to a pipe in a background thread, this will write a lot
|
|
+ # unless Rack (properly) shuts down the read end
|
|
+ thr = Thread.new do
|
|
+ begin
|
|
+ wr.write("--AaB03x")
|
|
+
|
|
+ # make the initial boundary a few gigs long
|
|
+ longer = "0123456789" * 1024 * 1024
|
|
+ (1024 * 1024).times { wr.write(longer) }
|
|
+
|
|
+ wr.write("\r\n")
|
|
+ wr.write('Content-Disposition: form-data; name="a"; filename="a.txt"')
|
|
+ wr.write("\r\n")
|
|
+ wr.write("Content-Type: text/plain\r\n")
|
|
+ wr.write("\r\na")
|
|
+ wr.write("--AaB03x--\r\n")
|
|
+ wr.close
|
|
+ rescue => err # this is EPIPE if Rack shuts us down
|
|
+ err
|
|
+ end
|
|
+ end
|
|
+
|
|
+ fixture = {
|
|
+ "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x",
|
|
+ "CONTENT_LENGTH" => rd.length.to_s,
|
|
+ :input => rd,
|
|
+ }
|
|
+
|
|
+ env = Rack::MockRequest.env_for '/', fixture
|
|
+ lambda {
|
|
+ Rack::Multipart.parse_multipart(env)
|
|
+ }.should.raise(EOFError)
|
|
+ rd.close
|
|
+
|
|
+ err = thr.value
|
|
+ err.should.be.instance_of Errno::EPIPE
|
|
+ wr.close
|
|
+ end
|
|
+
|
|
should "parse multipart upload with text file" do
|
|
env = Rack::MockRequest.env_for("/", multipart_fixture(:text))
|
|
params = Rack::Multipart.parse_multipart(env)
|
|
--
|
|
1.7.10
|
|
|
