From be5a83e84a34091f2a4e3c6dfb911b20e78e690c Mon Sep 17 00:00:00 2001 From: usa Date: Wed, 7 Jul 2021 10:34:08 +0000 Subject: Ignore IP addresses in PASV responses by default, and add new option use_pasv_ip This fixes CVE-2021-31810. Reported by Alexandr Savca. Co-authored-by: Shugo Maeda git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_6@67949 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- 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 e68d825dcf..c5d669d898 100644 --- a/lib/net/ftp.rb +++ b/lib/net/ftp.rb @@ -97,6 +97,10 @@ module Net # 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 # 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 @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] @@ -1330,7 +1338,12 @@ module Net raise FTPReplyError, resp end if m = /\((?\d+(,\d+){3}),(?\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 a5219644bb..b3fe7774ed 100644 --- a/test/net/ftp/test_ftp.rb +++ b/test/net/ftp/test_ftp.rb @@ -61,7 +61,7 @@ class FTPTest < Test::Unit::TestCase 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 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 @@ -2360,10 +2368,155 @@ EOF 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 3e71b2c51d..668fd5da22 100644 --- a/version.h +++ b/version.h @@ -1,10 +1,10 @@ #define RUBY_VERSION "2.6.8" -#define RUBY_RELEASE_DATE "2021-07-04" -#define RUBY_PATCHLEVEL 203 +#define RUBY_RELEASE_DATE "2021-07-07" +#define RUBY_PATCHLEVEL 204 #define RUBY_RELEASE_YEAR 2021 #define RUBY_RELEASE_MONTH 7 -#define RUBY_RELEASE_DAY 4 +#define RUBY_RELEASE_DAY 7 #include "ruby/version.h" -- cgit v1.2.3