Merge pull request #301 from ruby/header-crlf-injection

Reject CR/LF in remaining request-generation paths

Author: hsbt

lib/net/http/generic_request.rb

@@ -336,6 +336,9 @@ def encode_multipart_form_data(out, params, opt)
     boundary = opt[:boundary]
     require 'securerandom' unless defined?(SecureRandom)
     boundary ||= SecureRandom.urlsafe_base64(40)
+    if /[\r\n]/.match?(boundary.to_s)
+      raise ArgumentError, "multipart boundary cannot include CR/LF"
+    end
     chunked_p = chunked?
 
     buf = +''
@@ -349,7 +352,10 @@ def encode_multipart_form_data(out, params, opt)
       buf << "--#{boundary}\r\n"
       if filename
         filename = quote_string(filename, charset)
-        type = h[:content_type] || 'application/octet-stream'
+        type = (h[:content_type] || 'application/octet-stream').to_s
+        if /[\r\n]/.match?(type)
+          raise ArgumentError, "field content type cannot include CR/LF"
+        end
         buf << "Content-Disposition: form-data; " \
           "name=\"#{key}\"; filename=\"#{filename}\"\r\n" \
           "Content-Type: #{type}\r\n\r\n"
@@ -384,6 +390,9 @@ def encode_multipart_form_data(out, params, opt)
 
   def quote_string(str, charset)
     str = str.encode(charset, fallback:->(c){'&#%d;'%c.encode("UTF-8").ord}) if charset
+    if /[\r\n]/.match?(str)
+      raise ArgumentError, "multipart field name or filename cannot include CR/LF"
+    end
     str.gsub(/[\\"]/, '\\\\\&')
   end
 

lib/net/http/header.rb

@@ -196,6 +196,7 @@ def initialize_http_header(initheader) #:nodoc:
         if key.to_s.bytesize > MAX_KEY_LENGTH
           raise ArgumentError, "too long (#{key.bytesize} bytes) header: #{key[0, 30].inspect}..."
         end
+        validate_field_name(key)
         if value.to_s.bytesize > MAX_FIELD_LENGTH
           raise ArgumentError, "header #{key} has too long field value: #{value.bytesize}"
         end
@@ -270,7 +271,14 @@ def add_field(key, val)
   end
 
   # :stopdoc:
+  private def validate_field_name(key)
+    if /[\x00-\x1f\x7f:]/n.match?(key.to_s.b)
+      raise ArgumentError, "header field name cannot include control characters or colon: #{key.to_s[0, 30].inspect}"
+    end
+  end
+
   private def set_field(key, val)
+    validate_field_name(key)
     case val
     when Enumerable
       ary = []
@@ -774,7 +782,7 @@ def type_params
   #
   # Net::HTTPHeader#content_type= is an alias for Net::HTTPHeader#set_content_type.
   def set_content_type(type, params = {})
-    @header['content-type'] = [type + params.map{|k,v|"; #{k}=#{v}"}.join('')]
+    set_field('Content-Type', type + params.map{|k,v|"; #{k}=#{v}"}.join(''))
   end
 
   alias content_type= set_content_type

test/net/http/test_http.rb

@@ -931,6 +931,24 @@ def test_set_form_with_file
       }
     }
   end
+
+  def test_set_form_multipart_crlf_injection
+    build = ->(data, opt = {}) {
+      req = Net::HTTP::Post.new('/')
+      req.set_form(data, 'multipart/form-data')
+      out = +''
+      req.send(:encode_multipart_form_data, out, req.instance_variable_get(:@body_data), opt)
+    }
+    assert_raise(ArgumentError) { build.call([["foo\r\nX-Injected: 1", 'v']]) }
+    assert_raise(ArgumentError) { build.call([['f', 'v']], boundary: "abc\r\nX-Injected: 1") }
+    assert_raise(ArgumentError) { build.call([['f', 'v', {filename: "a\r\nX-Injected: 1"}]]) }
+    assert_raise(ArgumentError) do
+      build.call([['f', 'v', {filename: 'a', content_type: "text/plain\r\nX-Injected: 1"}]])
+    end
+    assert_nothing_raised do
+      build.call([['f', 'v', {filename: 'a', content_type: :"text/plain"}]])
+    end
+  end
 end
 
 class TestNetHTTP_v1_2 < Test::Unit::TestCase

test/net/http/test_httpheader.rb

@@ -30,6 +30,19 @@ def test_initialize
     assert_raise(ArgumentError){ @c.initialize_http_header("foo"=>"a\rb") }
   end
 
+  def test_invalid_field_name
+    assert_raise(ArgumentError){ @c.initialize_http_header("foo\nbar"=>"abc") }
+    assert_raise(ArgumentError){ @c.initialize_http_header("foo\rbar"=>"abc") }
+    assert_raise(ArgumentError){ @c.initialize_http_header("foo:bar"=>"abc") }
+    assert_raise(ArgumentError){ @c.initialize_http_header("foo\x00bar"=>"abc") }
+    assert_raise(ArgumentError){ @c['foo'.b << 0x0a << 'bar'] = 'abc' }
+    assert_raise(ArgumentError){ @c["foo\rbar"] = 'abc' }
+    assert_raise(ArgumentError){ @c["foo:bar"] = 'abc' }
+    assert_raise(ArgumentError){ @c["foo\x7fbar"] = 'abc' }
+    assert_raise(ArgumentError){ @c.add_field "foo\nbar", 'abc' }
+    assert_raise(ArgumentError){ @c.add_field "foo\nbar", ['abc'] }
+  end
+
   def test_initialize_with_broken_coderange
     error = RUBY_VERSION >= "3.2" ? Encoding::CompatibilityError : ArgumentError
     assert_raise(error){ @c.initialize_http_header("foo"=>"a\xff") }
@@ -438,6 +451,11 @@ def test_type_params
   end
 
   def test_set_content_type
+    @c.set_content_type 'text/html', {'charset' => 'utf-8'}
+    assert_equal 'text/html; charset=utf-8', @c['content-type']
+    assert_raise(ArgumentError){ @c.set_content_type "text/html\r\nFoo: bar" }
+    assert_raise(ArgumentError){ @c.set_content_type 'text/html', {'charset' => "x\r\nFoo: bar"} }
+    assert_raise(ArgumentError){ @c.set_content_type 'text/html', {"x\nFoo: bar" => 'utf-8'} }
   end
 
   def test_form_data=