ruby-changes:66717
From: Yusuke <ko1@a...>
Date: Wed, 7 Jul 2021 19:49:37 +0900 (JST)
Subject: [ruby-changes:66717] 3ca1399150 (ruby_2_7): Ignore IP addresses in PASV responses by default, and add new option use_pasv_ip
https://git.ruby-lang.org/ruby.git/commit/?id=3ca1399150 From 3ca1399150ed4eacfd2fe1ee251b966f8d1ee469 Mon Sep 17 00:00:00 2001 From: Yusuke Endoh <mame@r...> Date: Wed, 7 Jul 2021 12:05:44 +0900 Subject: Ignore IP addresses in PASV responses by default, and add new option use_pasv_ip This fixes CVE-2021-81810. Reported by Alexandr Savca. Co-authored-by: Shugo Maeda <shugo@r...> --- lib/net/ftp.rb | 15 ++++- test/net/ftp/test_ftp.rb | 159 ++++++++++++++++++++++++++++++++++++++++++++++- version.h | 6 +- 3 files changed, 173 insertions(+), 7 deletions(-) diff --git a/lib/net/ftp.rb b/lib/net/ftp.rb index 610027d..1e07845 100644 --- a/lib/net/ftp.rb +++ b/lib/net/ftp.rb @@ -97,6 +97,10 @@ module Net https://github.com/ruby/ruby/blob/trunk/lib/net/ftp.rb#L97 # When +true+, the connection is in passive mode. Default: +true+. attr_accessor :passive + # When +true+, use the IP address in PASV responses. Otherwise, it uses + # the same IP address for the control connection. Default: +false+. + attr_accessor :use_pasv_ip + # When +true+, all traffic to and from the server is written # to +$stdout+. Default: +false+. attr_accessor :debug_mode @@ -205,6 +209,9 @@ module Net https://github.com/ruby/ruby/blob/trunk/lib/net/ftp.rb#L209 # handshake. # See Net::FTP#ssl_handshake_timeout for # details. Default: +nil+. + # use_pasv_ip:: When +true+, use the IP address in PASV responses. + # Otherwise, it uses the same IP address for the control + # connection. Default: +false+. # debug_mode:: When +true+, all traffic to and from the server is # written to +$stdout+. Default: +false+. # @@ -265,6 +272,7 @@ module Net https://github.com/ruby/ruby/blob/trunk/lib/net/ftp.rb#L272 @open_timeout = options[:open_timeout] @ssl_handshake_timeout = options[:ssl_handshake_timeout] @read_timeout = options[:read_timeout] || 60 + @use_pasv_ip = options[:use_pasv_ip] || false if host connect(host, options[:port] || FTP_PORT) if options[:username] @@ -1370,7 +1378,12 @@ module Net https://github.com/ruby/ruby/blob/trunk/lib/net/ftp.rb#L1378 raise FTPReplyError, resp end if m = /\((?<host>\d+(,\d+){3}),(?<port>\d+,\d+)\)/.match(resp) - return parse_pasv_ipv4_host(m["host"]), parse_pasv_port(m["port"]) + if @use_pasv_ip + host = parse_pasv_ipv4_host(m["host"]) + else + host = @bare_sock.remote_address.ip_address + end + return host, parse_pasv_port(m["port"]) else raise FTPProtoError, resp end diff --git a/test/net/ftp/test_ftp.rb b/test/net/ftp/test_ftp.rb index 2504a48..24c5d3a 100644 --- a/test/net/ftp/test_ftp.rb +++ b/test/net/ftp/test_ftp.rb @@ -61,7 +61,7 @@ class FTPTest < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/net/ftp/test_ftp.rb#L61 end def test_parse227 - ftp = Net::FTP.new + ftp = Net::FTP.new(nil, use_pasv_ip: true) host, port = ftp.send(:parse227, "227 Entering Passive Mode (192,168,0,1,12,34)") assert_equal("192.168.0.1", host) assert_equal(3106, port) @@ -80,6 +80,14 @@ class FTPTest < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/net/ftp/test_ftp.rb#L80 assert_raise(Net::FTPProtoError) do ftp.send(:parse227, "227 ) foo bar (") end + + ftp = Net::FTP.new + sock = OpenStruct.new + sock.remote_address = OpenStruct.new + sock.remote_address.ip_address = "10.0.0.1" + ftp.instance_variable_set(:@bare_sock, sock) + host, port = ftp.send(:parse227, "227 Entering Passive Mode (192,168,0,1,12,34)") + assert_equal("10.0.0.1", host) end def test_parse228 @@ -2459,10 +2467,155 @@ EOF https://github.com/ruby/ruby/blob/trunk/test/net/ftp/test_ftp.rb#L2467 end end + def test_ignore_pasv_ip + commands = [] + binary_data = (0..0xff).map {|i| i.chr}.join * 4 * 3 + server = create_ftp_server(nil, "127.0.0.1") { |sock| + sock.print("220 (test_ftp).\r\n") + commands.push(sock.gets) + sock.print("331 Please specify the password.\r\n") + commands.push(sock.gets) + sock.print("230 Login successful.\r\n") + commands.push(sock.gets) + sock.print("200 Switching to Binary mode.\r\n") + line = sock.gets + commands.push(line) + data_server = TCPServer.new("127.0.0.1", 0) + port = data_server.local_address.ip_port + sock.printf("227 Entering Passive Mode (999,0,0,1,%s).\r\n", + port.divmod(256).join(",")) + commands.push(sock.gets) + sock.print("150 Opening BINARY mode data connection for foo (#{binary_data.size} bytes)\r\n") + conn = data_server.accept + binary_data.scan(/.{1,1024}/nm) do |s| + conn.print(s) + end + conn.shutdown(Socket::SHUT_WR) + conn.read + conn.close + data_server.close + sock.print("226 Transfer complete.\r\n") + } + begin + begin + ftp = Net::FTP.new + ftp.passive = true + ftp.read_timeout *= 5 if defined?(RubyVM::MJIT) && RubyVM::MJIT.enabled? # for --jit-wait + ftp.connect("127.0.0.1", server.port) + ftp.login + assert_match(/\AUSER /, commands.shift) + assert_match(/\APASS /, commands.shift) + assert_equal("TYPE I\r\n", commands.shift) + buf = ftp.getbinaryfile("foo", nil) + assert_equal(binary_data, buf) + assert_equal(Encoding::ASCII_8BIT, buf.encoding) + assert_equal("PASV\r\n", commands.shift) + assert_equal("RETR foo\r\n", commands.shift) + assert_equal(nil, commands.shift) + ensure + ftp.close if ftp + end + ensure + server.close + end + end + + def test_use_pasv_ip + commands = [] + binary_data = (0..0xff).map {|i| i.chr}.join * 4 * 3 + server = create_ftp_server(nil, "127.0.0.1") { |sock| + sock.print("220 (test_ftp).\r\n") + commands.push(sock.gets) + sock.print("331 Please specify the password.\r\n") + commands.push(sock.gets) + sock.print("230 Login successful.\r\n") + commands.push(sock.gets) + sock.print("200 Switching to Binary mode.\r\n") + line = sock.gets + commands.push(line) + data_server = TCPServer.new("127.0.0.1", 0) + port = data_server.local_address.ip_port + sock.printf("227 Entering Passive Mode (127,0,0,1,%s).\r\n", + port.divmod(256).join(",")) + commands.push(sock.gets) + sock.print("150 Opening BINARY mode data connection for foo (#{binary_data.size} bytes)\r\n") + conn = data_server.accept + binary_data.scan(/.{1,1024}/nm) do |s| + conn.print(s) + end + conn.shutdown(Socket::SHUT_WR) + conn.read + conn.close + data_server.close + sock.print("226 Transfer complete.\r\n") + } + begin + begin + ftp = Net::FTP.new + ftp.passive = true + ftp.use_pasv_ip = true + ftp.read_timeout *= 5 if defined?(RubyVM::MJIT) && RubyVM::MJIT.enabled? # for --jit-wait + ftp.connect("127.0.0.1", server.port) + ftp.login + assert_match(/\AUSER /, commands.shift) + assert_match(/\APASS /, commands.shift) + assert_equal("TYPE I\r\n", commands.shift) + buf = ftp.getbinaryfile("foo", nil) + assert_equal(binary_data, buf) + assert_equal(Encoding::ASCII_8BIT, buf.encoding) + assert_equal("PASV\r\n", commands.shift) + assert_equal("RETR foo\r\n", commands.shift) + assert_equal(nil, commands.shift) + ensure + ftp.close if ftp + end + ensure + server.close + end + end + + def test_use_pasv_invalid_ip + commands = [] + binary_data = (0..0xff).map {|i| i.chr}.join * 4 * 3 + server = create_ftp_server(nil, "127.0.0.1") { |sock| + sock.print("220 (test_ftp).\r\n") + commands.push(sock.gets) + sock.print("331 Please specify the password.\r\n") + commands.push(sock.gets) + sock.print("230 Login successful.\r\n") + commands.push(sock.gets) + sock.print("200 Switching to Binary mode.\r\n") + line = sock.gets + commands.push(line) + sock.print("227 Entering Passive Mode (999,0,0,1,48,57).\r\n") + commands.push(sock.gets) + } + begin + begin + ftp = Net::FTP.new + ftp.passive = true + ftp.use_pasv_ip = true + ftp.read_timeout *= 5 if defined?(RubyVM::MJIT) && RubyVM::MJIT.enabled? # for --jit-wait + ftp.connect("127.0.0.1", server.port) + ftp.login + assert_match(/\AUSER /, commands.shift) + assert_match(/\APASS /, commands.shift) + assert_equal("TYPE I\r\n", commands.shift) + assert_raise(SocketError) do + ftp.getbinaryfile("foo", nil) + end + ensure + ftp.close if ftp + end + ensure + server.close + end + end + private - def create_ftp_server(sleep_time = nil) - server = TCPServer.new(SERVER_ADDR, 0) + def create_ftp_server(sleep_time = nil, addr = SERVER_ADDR) + server = TCPServer.new(addr, 0) @thread = Thread.start do if sleep_time sleep(sleep_time) diff --git a/version.h b/version.h index 72bf707..f41251e 100644 --- a/version.h +++ b/version.h @@ -2,11 +2,11 @@ https://github.com/ruby/ruby/blob/trunk/version.h#L2 # define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR #define RUBY_VERSION_TEENY 4 #define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR -#define RUBY_PATCHLEVEL 189 +#define RUBY_PATCHLEVEL 190 #define RUBY_RELEASE_YEAR 2021 -#define RUBY_RELEASE_MONTH 6 -#define RUBY_RELEASE_DAY 2 +#define RUBY_RELEASE_MONTH 7 +#define RUBY_RELEASE_DAY 7 #include "ruby/version.h" -- cgit v1.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/