Skip to content

Commit 6d3f72e

Browse files
nagachikanagachika
committed
Fix a command injection vulnerability in Net::FTP.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@61242 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
1 parent 2e315ba commit 6d3f72e

File tree

2 files changed

+239
-5
lines changed

2 files changed

+239
-5
lines changed

‎lib/net/ftp.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -748,10 +748,10 @@ def getbinaryfile(remotefile, localfile = File.basename(remotefile),
748748
if localfile
749749
if @resume
750750
rest_offset = File.size?(localfile)
751-
f = open(localfile, "a")
751+
f = File.open(localfile, "a")
752752
else
753753
rest_offset = nil
754-
f = open(localfile, "w")
754+
f = File.open(localfile, "w")
755755
end
756756
elsif !block_given?
757757
result = String.new
@@ -781,7 +781,7 @@ def gettextfile(remotefile, localfile = File.basename(remotefile),
781781
f = nil
782782
result = nil
783783
if localfile
784-
f = open(localfile, "w")
784+
f = File.open(localfile, "w")
785785
elsif !block_given?
786786
result = String.new
787787
end
@@ -827,7 +827,7 @@ def putbinaryfile(localfile, remotefile = File.basename(localfile),
827827
else
828828
rest_offset = nil
829829
end
830-
f = open(localfile)
830+
f = File.open(localfile)
831831
begin
832832
f.binmode
833833
if rest_offset
@@ -846,7 +846,7 @@ def putbinaryfile(localfile, remotefile = File.basename(localfile),
846846
# passing in the transmitted data one line at a time.
847847
#
848848
def puttextfile(localfile, remotefile = File.basename(localfile), &block) # :yield: line
849-
f = open(localfile)
849+
f = File.open(localfile)
850850
begin
851851
storlines("STOR #{remotefile}", f, &block)
852852
ensure

‎test/net/ftp/test_ftp.rb

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
require "ostruct"
66
require "stringio"
77
require "tempfile"
8+
require "tmpdir"
89

910
class FTPTest < Test::Unit::TestCase
1011
SERVER_NAME = "localhost"
@@ -2125,6 +2126,227 @@ def test_abort_tls
21252126
end
21262127
end
21272128

2129+
def test_getbinaryfile_command_injection
2130+
skip "| is not allowed in filename on Windows" if windows?
2131+
[false, true].each do |resume|
2132+
commands = []
2133+
binary_data = (0..0xff).map {|i| i.chr}.join * 4 * 3
2134+
server = create_ftp_server { |sock|
2135+
sock.print("220 (test_ftp).\r\n")
2136+
commands.push(sock.gets)
2137+
sock.print("331 Please specify the password.\r\n")
2138+
commands.push(sock.gets)
2139+
sock.print("230 Login successful.\r\n")
2140+
commands.push(sock.gets)
2141+
sock.print("200 Switching to Binary mode.\r\n")
2142+
line = sock.gets
2143+
commands.push(line)
2144+
host, port = process_port_or_eprt(sock, line)
2145+
commands.push(sock.gets)
2146+
sock.print("150 Opening BINARY mode data connection for |echo hello (#{binary_data.size} bytes)\r\n")
2147+
conn = TCPSocket.new(host, port)
2148+
binary_data.scan(/.{1,1024}/nm) do |s|
2149+
conn.print(s)
2150+
end
2151+
conn.shutdown(Socket::SHUT_WR)
2152+
conn.read
2153+
conn.close
2154+
sock.print("226 Transfer complete.\r\n")
2155+
}
2156+
begin
2157+
chdir_to_tmpdir do
2158+
begin
2159+
ftp = Net::FTP.new
2160+
ftp.resume = resume
2161+
ftp.read_timeout = 0.2
2162+
ftp.connect(SERVER_ADDR, server.port)
2163+
ftp.login
2164+
assert_match(/\AUSER /, commands.shift)
2165+
assert_match(/\APASS /, commands.shift)
2166+
assert_equal("TYPE I\r\n", commands.shift)
2167+
ftp.getbinaryfile("|echo hello")
2168+
assert_equal(binary_data, File.binread("./|echo hello"))
2169+
assert_match(/\A(PORT|EPRT) /, commands.shift)
2170+
assert_equal("RETR |echo hello\r\n", commands.shift)
2171+
assert_equal(nil, commands.shift)
2172+
ensure
2173+
ftp.close if ftp
2174+
end
2175+
end
2176+
ensure
2177+
server.close
2178+
end
2179+
end
2180+
end
2181+
2182+
def test_gettextfile_command_injection
2183+
skip "| is not allowed in filename on Windows" if windows?
2184+
commands = []
2185+
text_data = <<EOF.gsub(/\n/, "\r\n")
2186+
foo
2187+
bar
2188+
baz
2189+
EOF
2190+
server = create_ftp_server { |sock|
2191+
sock.print("220 (test_ftp).\r\n")
2192+
commands.push(sock.gets)
2193+
sock.print("331 Please specify the password.\r\n")
2194+
commands.push(sock.gets)
2195+
sock.print("230 Login successful.\r\n")
2196+
commands.push(sock.gets)
2197+
sock.print("200 Switching to Binary mode.\r\n")
2198+
commands.push(sock.gets)
2199+
sock.print("200 Switching to ASCII mode.\r\n")
2200+
line = sock.gets
2201+
commands.push(line)
2202+
host, port = process_port_or_eprt(sock, line)
2203+
commands.push(sock.gets)
2204+
sock.print("150 Opening TEXT mode data connection for |echo hello (#{text_data.size} bytes)\r\n")
2205+
conn = TCPSocket.new(host, port)
2206+
text_data.each_line do |l|
2207+
conn.print(l)
2208+
end
2209+
conn.shutdown(Socket::SHUT_WR)
2210+
conn.read
2211+
conn.close
2212+
sock.print("226 Transfer complete.\r\n")
2213+
commands.push(sock.gets)
2214+
sock.print("200 Switching to Binary mode.\r\n")
2215+
}
2216+
begin
2217+
chdir_to_tmpdir do
2218+
begin
2219+
ftp = Net::FTP.new
2220+
ftp.connect(SERVER_ADDR, server.port)
2221+
ftp.login
2222+
assert_match(/\AUSER /, commands.shift)
2223+
assert_match(/\APASS /, commands.shift)
2224+
assert_equal("TYPE I\r\n", commands.shift)
2225+
ftp.gettextfile("|echo hello")
2226+
assert_equal(text_data.gsub(/\r\n/, "\n"),
2227+
File.binread("./|echo hello"))
2228+
assert_equal("TYPE A\r\n", commands.shift)
2229+
assert_match(/\A(PORT|EPRT) /, commands.shift)
2230+
assert_equal("RETR |echo hello\r\n", commands.shift)
2231+
assert_equal("TYPE I\r\n", commands.shift)
2232+
assert_equal(nil, commands.shift)
2233+
ensure
2234+
ftp.close if ftp
2235+
end
2236+
end
2237+
ensure
2238+
server.close
2239+
end
2240+
end
2241+
2242+
def test_putbinaryfile_command_injection
2243+
skip "| is not allowed in filename on Windows" if windows?
2244+
commands = []
2245+
binary_data = (0..0xff).map {|i| i.chr}.join * 4 * 3
2246+
received_data = nil
2247+
server = create_ftp_server { |sock|
2248+
sock.print("220 (test_ftp).\r\n")
2249+
commands.push(sock.gets)
2250+
sock.print("331 Please specify the password.\r\n")
2251+
commands.push(sock.gets)
2252+
sock.print("230 Login successful.\r\n")
2253+
commands.push(sock.gets)
2254+
sock.print("200 Switching to Binary mode.\r\n")
2255+
line = sock.gets
2256+
commands.push(line)
2257+
host, port = process_port_or_eprt(sock, line)
2258+
commands.push(sock.gets)
2259+
sock.print("150 Opening BINARY mode data connection for |echo hello (#{binary_data.size} bytes)\r\n")
2260+
conn = TCPSocket.new(host, port)
2261+
received_data = conn.read
2262+
conn.close
2263+
sock.print("226 Transfer complete.\r\n")
2264+
}
2265+
begin
2266+
chdir_to_tmpdir do
2267+
File.binwrite("./|echo hello", binary_data)
2268+
begin
2269+
ftp = Net::FTP.new
2270+
ftp.read_timeout = 0.2
2271+
ftp.connect(SERVER_ADDR, server.port)
2272+
ftp.login
2273+
assert_match(/\AUSER /, commands.shift)
2274+
assert_match(/\APASS /, commands.shift)
2275+
assert_equal("TYPE I\r\n", commands.shift)
2276+
ftp.putbinaryfile("|echo hello")
2277+
assert_equal(binary_data, received_data)
2278+
assert_match(/\A(PORT|EPRT) /, commands.shift)
2279+
assert_equal("STOR |echo hello\r\n", commands.shift)
2280+
assert_equal(nil, commands.shift)
2281+
ensure
2282+
ftp.close if ftp
2283+
end
2284+
end
2285+
ensure
2286+
server.close
2287+
end
2288+
end
2289+
2290+
def test_puttextfile_command_injection
2291+
skip "| is not allowed in filename on Windows" if windows?
2292+
commands = []
2293+
received_data = nil
2294+
server = create_ftp_server { |sock|
2295+
sock.print("220 (test_ftp).\r\n")
2296+
commands.push(sock.gets)
2297+
sock.print("331 Please specify the password.\r\n")
2298+
commands.push(sock.gets)
2299+
sock.print("230 Login successful.\r\n")
2300+
commands.push(sock.gets)
2301+
sock.print("200 Switching to Binary mode.\r\n")
2302+
commands.push(sock.gets)
2303+
sock.print("200 Switching to ASCII mode.\r\n")
2304+
line = sock.gets
2305+
commands.push(line)
2306+
host, port = process_port_or_eprt(sock, line)
2307+
commands.push(sock.gets)
2308+
sock.print("150 Opening TEXT mode data connection for |echo hello\r\n")
2309+
conn = TCPSocket.new(host, port)
2310+
received_data = conn.read
2311+
conn.close
2312+
sock.print("226 Transfer complete.\r\n")
2313+
commands.push(sock.gets)
2314+
sock.print("200 Switching to Binary mode.\r\n")
2315+
}
2316+
begin
2317+
chdir_to_tmpdir do
2318+
File.open("|echo hello", "w") do |f|
2319+
f.puts("foo")
2320+
f.puts("bar")
2321+
f.puts("baz")
2322+
end
2323+
begin
2324+
ftp = Net::FTP.new
2325+
ftp.connect(SERVER_ADDR, server.port)
2326+
ftp.login
2327+
assert_match(/\AUSER /, commands.shift)
2328+
assert_match(/\APASS /, commands.shift)
2329+
assert_equal("TYPE I\r\n", commands.shift)
2330+
ftp.puttextfile("|echo hello")
2331+
assert_equal(<<EOF.gsub(/\n/, "\r\n"), received_data)
2332+
foo
2333+
bar
2334+
baz
2335+
EOF
2336+
assert_equal("TYPE A\r\n", commands.shift)
2337+
assert_match(/\A(PORT|EPRT) /, commands.shift)
2338+
assert_equal("STOR |echo hello\r\n", commands.shift)
2339+
assert_equal("TYPE I\r\n", commands.shift)
2340+
assert_equal(nil, commands.shift)
2341+
ensure
2342+
ftp.close if ftp
2343+
end
2344+
end
2345+
ensure
2346+
server.close
2347+
end
2348+
end
2349+
21282350
private
21292351

21302352
def create_ftp_server(sleep_time = nil)
@@ -2221,4 +2443,16 @@ def create_data_connection_server(sock)
22212443
end
22222444
return data_server
22232445
end
2446+
2447+
def chdir_to_tmpdir
2448+
Dir.mktmpdir do |dir|
2449+
pwd = Dir.pwd
2450+
Dir.chdir(dir)
2451+
begin
2452+
yield
2453+
ensure
2454+
Dir.chdir(pwd)
2455+
end
2456+
end
2457+
end
22242458
end

0 commit comments

Comments
 (0)