From 0207c68ea39b74fc99e445231c1ac08ad5406720 Mon Sep 17 00:00:00 2001
From: usa <usa@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>
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 <shugo@ruby-lang.org>
+
+ Fix a command injection vulnerability in Net::FTP.
+
Tue Nov 15 15:29:36 2016 NARUSE, Yui <naruse@ruby-lang.org>
* 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 = <<EOF.gsub(/\n/, "\r\n")
+foo
+bar
+baz
+EOF
+ 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")
+ commands.push(sock.gets)
+ sock.print("200 Switching to ASCII 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 TEXT mode data connection for |echo hello (#{text_data.size} bytes)\r\n")
+ conn = TCPSocket.new(host, port)
+ text_data.each_line do |l|
+ conn.print(l)
+ end
+ conn.shutdown(Socket::SHUT_WR)
+ conn.read
+ conn.close
+ sock.print("226 Transfer complete.\r\n")
+ commands.push(sock.gets)
+ sock.print("200 Switching to Binary mode.\r\n")
+ }
+ begin
+ chdir_to_tmpdir do
+ begin
+ ftp = Net::FTP.new
+ 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.gettextfile("|echo hello")
+ assert_equal(text_data.gsub(/\r\n/, "\n"),
+ File.binread("./|echo hello"))
+ assert_equal("TYPE A\r\n", commands.shift)
+ assert_match(/\A(PORT|EPRT) /, commands.shift)
+ assert_equal("RETR |echo hello\r\n", commands.shift)
+ assert_equal("TYPE I\r\n", commands.shift)
+ assert_equal(nil, commands.shift)
+ ensure
+ ftp.close if ftp
+ end
+ end
+ ensure
+ server.close
+ end
+ end
+
+ def test_putbinaryfile_command_injection
+ skip "| is not allowed in filename on Windows" if windows?
+ commands = []
+ binary_data = (0..0xff).map {|i| i.chr}.join * 4 * 3
+ received_data = nil
+ 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)
+ received_data = conn.read
+ conn.close
+ sock.print("226 Transfer complete.\r\n")
+ }
+ begin
+ chdir_to_tmpdir do
+ File.binwrite("./|echo hello", binary_data)
+ begin
+ ftp = Net::FTP.new
+ 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.putbinaryfile("|echo hello")
+ assert_equal(binary_data, received_data)
+ assert_match(/\A(PORT|EPRT) /, commands.shift)
+ assert_equal("STOR |echo hello\r\n", commands.shift)
+ assert_equal(nil, commands.shift)
+ ensure
+ ftp.close if ftp
+ end
+ end
+ ensure
+ server.close
+ end
+ end
+
+ def test_puttextfile_command_injection
+ skip "| is not allowed in filename on Windows" if windows?
+ commands = []
+ received_data = nil
+ 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")
+ commands.push(sock.gets)
+ sock.print("200 Switching to ASCII 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 TEXT mode data connection for |echo hello\r\n")
+ conn = TCPSocket.new(host, port)
+ received_data = conn.read
+ conn.close
+ sock.print("226 Transfer complete.\r\n")
+ commands.push(sock.gets)
+ sock.print("200 Switching to Binary mode.\r\n")
+ }
+ begin
+ chdir_to_tmpdir do
+ File.open("|echo hello", "w") do |f|
+ f.puts("foo")
+ f.puts("bar")
+ f.puts("baz")
+ end
+ begin
+ ftp = Net::FTP.new
+ 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.puttextfile("|echo hello")
+ assert_equal(<<EOF.gsub(/\n/, "\r\n"), received_data)
+foo
+bar
+baz
+EOF
+ assert_equal("TYPE A\r\n", commands.shift)
+ assert_match(/\A(PORT|EPRT) /, commands.shift)
+ assert_equal("STOR |echo hello\r\n", commands.shift)
+ assert_equal("TYPE I\r\n", commands.shift)
+ assert_equal(nil, commands.shift)
+ ensure
+ ftp.close if ftp
+ end
+ end
+ ensure
+ server.close
+ end
+ end
+
private
@@ -810,4 +1032,16 @@ class FTPTest < Test::Unit::TestCase
end
return server
end
+
+ def chdir_to_tmpdir
+ Dir.mktmpdir do |dir|
+ pwd = Dir.pwd
+ Dir.chdir(dir)
+ begin
+ yield
+ ensure
+ Dir.chdir(pwd)
+ end
+ end
+ end
end
--
2.15.1
From 02b8978ff10b05304dbb46d73b49a2cf3a87cb92 Mon Sep 17 00:00:00 2001
From: usa <usa@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>
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 <usa@ruby-lang.org>
+
+ * 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 <shugo@ruby-lang.org>
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