Skip to content

Commit

Permalink
merge revision(s) 61242: [Backport #14185]
Browse files Browse the repository at this point in the history 
	Fix a command injection vulnerability in Net::FTP.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_4@61245 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
  • Loading branch information
@nagachika
nagachika committed Dec 14, 2017
1 parent 2e728d5 commit 95645f5
Show file tree
Hide file tree
Showing 3 changed files with 240 additions and 6 deletions.
10 changes: 5 additions & 5 deletions lib/net/ftp.rb
View file
Expand Up @@ -754,10 +754,10 @@ def getbinaryfile(remotefile, localfile = File.basename(remotefile),
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 = String.new
Expand Down Expand Up @@ -787,7 +787,7 @@ def gettextfile(remotefile, localfile = File.basename(remotefile),
f = nil
result = nil
if localfile
f = open(localfile, "w")
f = File.open(localfile, "w")
elsif !block_given?
result = String.new
end
Expand Down Expand Up @@ -833,7 +833,7 @@ def putbinaryfile(localfile, remotefile = File.basename(localfile),
else
rest_offset = nil
end
f = open(localfile)
f = File.open(localfile)
begin
f.binmode
if rest_offset
Expand All @@ -852,7 +852,7 @@ def putbinaryfile(localfile, remotefile = File.basename(localfile),
# 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
Expand Down
234 changes: 234 additions & 0 deletions test/net/ftp/test_ftp.rb
View file
Expand Up @@ -5,6 +5,7 @@
require "ostruct"
require "stringio"
require "tempfile"
require "tmpdir"

class FTPTest < Test::Unit::TestCase
SERVER_NAME = "localhost"
Expand Down Expand Up @@ -2132,6 +2133,227 @@ def test_abort_tls
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

def create_ftp_server(sleep_time = nil)
Expand Down Expand Up @@ -2228,4 +2450,16 @@ def create_data_connection_server(sock)
end
return data_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 changes: 1 addition & 1 deletion version.h
View file
@@ -1,6 +1,6 @@
#define RUBY_VERSION "2.4.3"
#define RUBY_RELEASE_DATE "2017-12-14"
#define RUBY_PATCHLEVEL 204
#define RUBY_PATCHLEVEL 205

#define RUBY_RELEASE_YEAR 2017
#define RUBY_RELEASE_MONTH 12
Expand Down

0 comments on commit 95645f5

Please sign in to comment.