diff --git a/lib/net/http/generic_request.rb b/lib/net/http/generic_request.rb index 5b01ea4a..96396b0a 100644 --- a/lib/net/http/generic_request.rb +++ b/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 diff --git a/lib/net/http/header.rb b/lib/net/http/header.rb index 797a3bea..dd132f85 100644 --- a/lib/net/http/header.rb +++ b/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 diff --git a/test/net/http/test_http.rb b/test/net/http/test_http.rb index 4e7fa227..477c3415 100644 --- a/test/net/http/test_http.rb +++ b/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 diff --git a/test/net/http/test_httpheader.rb b/test/net/http/test_httpheader.rb index 69563168..afe96c14 100644 --- a/test/net/http/test_httpheader.rb +++ b/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=