CVE-2013-0183: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: multipart/parser.rb in Rack 1.3.x before 1.3.8 and 1.4.x before 1.4.3 allows remote attackers to cause a denial of service (memory consumption and out-of-memory error) via a long string in a Multipart HTTP packet.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
Published: 2013-03-01T05:40Z
Repository: https://github.com/rack/rack
Language: Ruby
Files changed: lib/rack/multipart/parser.rb, test/spec_multipart.rb
Commit date: 2013-01-07 10:17:35 -0800
Commit message: multipart/parser: avoid unbounded #gets method

Malicious clients may send excessively long lines
to trigger out-of-memory errors in a Rack web server.

Diff: 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
@@ -68,13 +68,20 @@ module Rack
         @rx ||= /(?:#{EOL})?#{Regexp.quote(@boundary)}(#{EOL}|--)/n
       end
 
       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
 
       def get_current_head_and_filename_and_content_type_and_name_and_body
         head = nil
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
@@ -46,10 +46,63 @@ describe Rack::Multipart do
     env['CONTENT_TYPE'] = "multipart/form-data; boundary=----WebKitFormBoundaryWLHCs9qmcJJoyjKR"
     params = Rack::Multipart.parse_multipart(env)
     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)
     params["submit-name"].should.equal "Larry"
     params["submit-name-with-content"].should.equal "Berry"

Vulnerable code:           read_buffer = @io.gets
          break if read_buffer == full_boundary
          raise EOFError, "bad content body" if read_buffer.nil?
Fixed code:           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
  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