Actions
Bug #17027
closedConnection leak possibility in Net::FTP#transfercmd
Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 2.7.1p83 (2020-03-31 revision a0c7c23c9c) [x86_64-darwin19]
Description
https://github.com/ruby/ruby/blob/bad7ab35d1e38f47b09f15fc5750387ac73b2286/lib/net/ftp.rb#L542-L556
https://github.com/ruby/net-ftp/blob/14d2544190f7e4b77b41a3fd0c676f5b8ebd238c/lib/net/ftp.rb#L542-L556
The connection conn
may not release if exception occurred.
Reproduce¶
$ docker run --rm \
--name vsftpd \
-p 20-21:20-21 \
-p 21100-21110:21100-21110 \
-e FTP_USER=user \
-e FTP_PASS=pass \
-e PASV_ADDRESS=localhost \
-e PASV_MIN_PORT=21100 \
-e PASV_MAX_PORT=21110 \
fauria/vsftpd
$ docker exec vsftpd ps aux | grep vsftp
root 1 0.0 0.0 11704 2580 ? Ss 09:44 0:00 /bin/bash /usr/sbin/run-vsftpd.sh
root 13 0.0 0.1 53296 3884 ? S 09:44 0:00 /usr/sbin/vsftpd /etc/vsftpd/vsftpd.conf
$ diff -u ~/.rbenv/versions/2.7.1/lib/ruby/2.7.0/net/ftp.rb.orig ~/.rbenv/versions/2.7.1/lib/ruby/2.7.0/net/ftp.rb.orig
--- /Users/koshigoe/.rbenv/versions/2.7.1/lib/ruby/2.7.0/net/ftp.rb.orig 2020-07-13 19:41:53.000000000 +0900
+++ /Users/koshigoe/.rbenv/versions/2.7.1/lib/ruby/2.7.0/net/ftp.rb 2020-07-13 19:42:46.000000000 +0900
@@ -549,6 +549,7 @@
end
end
resp = sendcmd(cmd)
+ raise
# skip 2XX for some ftp servers
resp = getresp if resp.start_with?("2")
if !resp.start_with?("1")
require 'net/ftp'
ftp = Net::FTP.new
ftp.passive = true
ftp.binary = true
ftp.connect('localhost')
ftp.login('user', 'pass')
begin
ftp.put(__FILE__, '/uploaded-bin')
rescue
ftp.close
sleep 300
end
$ docker exec vsftpd ps aux | grep vsftp
root 1 0.0 0.0 11704 2580 ? Ss 09:44 0:00 /bin/bash /usr/sbin/run-vsftpd.sh
root 13 0.0 0.1 53296 3884 ? S 09:44 0:00 /usr/sbin/vsftpd /etc/vsftpd/vsftpd.conf
nobody 191 0.0 0.1 75752 4420 ? Ss 10:43 0:00 /usr/sbin/vsftpd /etc/vsftpd/vsftpd.conf
ftp 193 0.0 0.1 75852 3768 ? S 10:43 0:00 /usr/sbin/vsftpd /etc/vsftpd/vsftpd.conf
Updated by koshigoe (Masataka SUZUKI) over 4 years ago
Updated by koshigoe (Masataka SUZUKI) over 4 years ago
Fixed only about ACTIVE mode?
Updated by koshigoe (Masataka SUZUKI) over 4 years ago
Is this patch correct?
Should I close connection use shutdown
and read
?
--- /Users/koshigoe/.rbenv/versions/2.7.1/lib/ruby/2.7.0/net/ftp.rb.orig 2020-07-13 19:41:53.000000000 +0900
+++ /Users/koshigoe/.rbenv/versions/2.7.1/lib/ruby/2.7.0/net/ftp.rb 2020-07-13 19:50:40.000000000 +0900
@@ -541,18 +541,23 @@
def transfercmd(cmd, rest_offset = nil) # :nodoc:
if @passive
host, port = makepasv
- conn = open_socket(host, port)
- if @resume and rest_offset
- resp = sendcmd("REST " + rest_offset.to_s)
- if !resp.start_with?("3")
+ begin
+ conn = open_socket(host, port)
+ if @resume and rest_offset
+ resp = sendcmd("REST " + rest_offset.to_s)
+ if !resp.start_with?("3")
+ raise FTPReplyError, resp
+ end
+ end
+ resp = sendcmd(cmd)
+ # skip 2XX for some ftp servers
+ resp = getresp if resp.start_with?("2")
+ if !resp.start_with?("1")
raise FTPReplyError, resp
end
- end
- resp = sendcmd(cmd)
- # skip 2XX for some ftp servers
- resp = getresp if resp.start_with?("2")
- if !resp.start_with?("1")
- raise FTPReplyError, resp
+ rescue
+ conn.close if conn
+ raise
end
else
sock = makeport
Updated by jeremyevans0 (Jeremy Evans) over 4 years ago
Your patch looks good to me. However, the net/ftp library is maintained in a separate repository. Please submit your patch as a pull request to https://github.com/ruby/net-ftp/pulls.
Updated by koshigoe (Masataka SUZUKI) over 4 years ago
Updated by jeremyevans0 (Jeremy Evans) over 3 years ago
- Status changed from Open to Closed
The original pull request was closed, but I committed a similar fix: https://github.com/ruby/net-ftp/pull/6
Actions
Like0
Like0Like0Like0Like0Like0Like0