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@ruby-lang.org>

3 files changed, 173 insertions, 7 deletions

diff --git a/lib/net/ftp.rb b/lib/net/ftp.rb
index 610027dc38..1e078453a8 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]
@@ -1370,7 +1378,12 @@ module Net
         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 2504a48d0a..24c5d3a12f 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
@@ -2459,10 +2467,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 72bf707852..f41251e06e 100644
--- a/version.h
+++ b/version.h
@@ -2,11 +2,11 @@
 # 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"