From 0207c68ea39b74fc99e445231c1ac08ad5406720 Mon Sep 17 00:00:00 2001 From: usa Date: Thu, 14 Dec 2017 13:53:48 +0000 Subject: [PATCH 1/2] merge revision(s) 61242: [Backport #14185] Fix a command injection vulnerability in Net::FTP. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_2@61246 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 4 + lib/net/ftp.rb | 10 +- test/net/ftp/test_ftp.rb | 234 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 243 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index 177ff95c8b..ecff5aff99 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +Thu Dec 14 22:52:11 2017 Shugo Maeda + + Fix a command injection vulnerability in Net::FTP. + Tue Nov 15 15:29:36 2016 NARUSE, Yui * ext/openssl/ossl_ssl.c (ssl_npn_select_cb_common): fix parsing diff --git a/lib/net/ftp.rb b/lib/net/ftp.rb index c9b80c6804..79edb80864 100644 --- a/lib/net/ftp.rb +++ b/lib/net/ftp.rb @@ -607,10 +607,10 @@ module Net if localfile if @resume rest_offset = File.size?(localfile) - f = open(localfile, "a") + f = File.open(localfile, "a") else rest_offset = nil - f = open(localfile, "w") + f = File.open(localfile, "w") end elsif !block_given? result = "" @@ -638,7 +638,7 @@ module Net def gettextfile(remotefile, localfile = File.basename(remotefile)) # :yield: line result = nil if localfile - f = open(localfile, "w") + f = File.open(localfile, "w") elsif !block_given? result = "" end @@ -684,7 +684,7 @@ module Net else rest_offset = nil end - f = open(localfile) + f = File.open(localfile) begin f.binmode if rest_offset @@ -703,7 +703,7 @@ module Net # passing in the transmitted data one line at a time. # def puttextfile(localfile, remotefile = File.basename(localfile), &block) # :yield: line - f = open(localfile) + f = File.open(localfile) begin storlines("STOR " + remotefile, f, &block) ensure diff --git a/test/net/ftp/test_ftp.rb b/test/net/ftp/test_ftp.rb index cb311695d0..91a6002c5c 100644 --- a/test/net/ftp/test_ftp.rb +++ b/test/net/ftp/test_ftp.rb @@ -2,6 +2,7 @@ require "net/ftp" require "test/unit" require "ostruct" require "stringio" +require "tmpdir" class FTPTest < Test::Unit::TestCase SERVER_ADDR = "127.0.0.1" @@ -783,6 +784,227 @@ class FTPTest < Test::Unit::TestCase end end + def test_getbinaryfile_command_injection + skip "| is not allowed in filename on Windows" if windows? + [false, true].each do |resume| + commands = [] + binary_data = (0..0xff).map {|i| i.chr}.join * 4 * 3 + server = create_ftp_server { |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) + host, port = process_port_or_eprt(sock, line) + commands.push(sock.gets) + sock.print("150 Opening BINARY mode data connection for |echo hello (#{binary_data.size} bytes)\r\n") + conn = TCPSocket.new(host, port) + binary_data.scan(/.{1,1024}/nm) do |s| + conn.print(s) + end + conn.shutdown(Socket::SHUT_WR) + conn.read + conn.close + sock.print("226 Transfer complete.\r\n") + } + begin + chdir_to_tmpdir do + begin + ftp = Net::FTP.new + ftp.resume = resume + ftp.read_timeout = 0.2 + ftp.connect(SERVER_ADDR, server.port) + ftp.login + assert_match(/\AUSER /, commands.shift) + assert_match(/\APASS /, commands.shift) + assert_equal("TYPE I\r\n", commands.shift) + ftp.getbinaryfile("|echo hello") + assert_equal(binary_data, File.binread("./|echo hello")) + assert_match(/\A(PORT|EPRT) /, commands.shift) + assert_equal("RETR |echo hello\r\n", commands.shift) + assert_equal(nil, commands.shift) + ensure + ftp.close if ftp + end + end + ensure + server.close + end + end + end + + def test_gettextfile_command_injection + skip "| is not allowed in filename on Windows" if windows? + commands = [] + text_data = < Date: Thu, 14 Dec 2017 15:08:49 +0000 Subject: [PATCH 2/2] * test/net/ftp/test_ftp.rb (process_port_or_eprt): merge a part of r56973 to pass the test introduced at previous commit. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_2@61255 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 5 +++++ test/net/ftp/test_ftp.rb | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 0 deletions(-) diff --git a/ChangeLog b/ChangeLog index ecff5aff99..d9d9629ffa 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +Fri Dec 15 00:08:26 2017 NAKAMURA Usaku + + * test/net/ftp/test_ftp.rb (process_port_or_eprt): merge a part of + r56973 to pass the test introduced at previous commit. + Thu Dec 14 22:52:11 2017 Shugo Maeda Fix a command injection vulnerability in Net::FTP. diff --git a/test/net/ftp/test_ftp.rb b/test/net/ftp/test_ftp.rb index 91a6002c5c..52e5873d61 100644 --- a/test/net/ftp/test_ftp.rb +++ b/test/net/ftp/test_ftp.rb @@ -1044,4 +1044,22 @@ EOF end end end + + def process_port_or_eprt(sock, line) + case line + when /\APORT (.*)/ + port_args = $1.split(/,/) + host = port_args[0, 4].join(".") + port = port_args[4, 2].map(&:to_i).inject {|x, y| (x << 8) + y} + sock.print("200 PORT command successful.\r\n") + return host, port + when /\AEPRT \|2\|(.*?)\|(.*?)\|/ + host = $1 + port = $2.to_i + sock.print("200 EPRT command successful.\r\n") + return host, port + else + flunk "PORT or EPRT expected" + end + end end -- 2.15.1