emilianobilli / pyftpdlib Goto Github PK

What steps will reproduce the problem?
1. svn checkout http://pyftpdlib.googlecode.com/svn/trunk/ pyftpdlib 

What is the expected output? What do you see instead?

Instead of a checked out copy of the source, I get an empty pyftpdlib
directory with only a .svn folder inside it

What version of the product are you using? On what operating system?
OS X client

Please provide any additional information below.
Revision 33 according to the checkout

Not a issue but a feature request. A cool thing is to manipulate transfered
files directly in Python instead on file system.

Is it possible to make anonymous write access explicitly configurable with
necessary security warnings all over the place?

I use the library as a quick way to pass files from and to virtual machines
running on the host in secured environment, so there is no need in password
complication. I would like to avoid patching libraries and typing extra
options besides basic "ftp get ftp://host/file"

It is not to be expected that all servers will necessarily support all of
the new commands defined in all future amendments to the FTP protocol.  In
order to permit clients to determine which new commands are supported by a
particular server, without trying each possible command, FEAT command was
added to the FTP command repertoire. It is described in RFC 2398 [1].

FEAT response must appear as the following:

211-Extensions supported:
 CMD1
 CMD2
 ...
211 End.

FEAT must be accepted also when the user is not yet authenticated.
Commands subsequent RFC959 implemented in the current pyftpdlib version are
MDTM and SIZE.

[1] http://www.networksorcery.com/enp/rfc/rfc2389.txt

In "serve_forever" method of "ftp_server" class "self.set_reuse_addr()" is
erroneously used *after* having tried to bind the ftpd socket and not *before*.
By calling asyncore.dispatcher object's set_reuse_addr() method we tell
ftpd to mark the socket as re-usable (setting the SO_REUSEADDR option).
Patch:


class ftp_server(asynchat.async_chat):
    """The base class for the backend."""

    def __init__(self, address, handler):
        asynchat.async_chat.__init__(self)
        self.address = address
        self.handler = handler
        self.create_socket(socket.AF_INET, socket.SOCK_STREAM)  

+        if os.name == 'posix':
+            self.set_reuse_addr()  

        self.bind(self.address)
        self.listen(5)

    def serve_forever(self): 
        log("Serving FTP on %s:%s." %self.socket.getsockname())

-         if os.name == 'posix':
-             self.set_reuse_addr()

        # here we try to use poll(), if it exists, else we'll use select()
        asyncore.loop(timeout=1, use_poll=True)    

What steps will reproduce the problem?
Sending a REST request followed by a STOU request.

What is the expected output? What do you see instead?
FTPd should watch for STOU preceded by REST, which makes no sense, in which
case it should reject REST request responding with some kind of error code
(e.g. 550).

What steps will reproduce the problem?
1. Connect to ftpd.
2. Send QUIT command before authenticating.

What is the expected output? What do you see instead?

The following exception is raised:

Traceback (most recent call last):
  File "asyncore.py", line 68, in read
    obj.handle_read_event()
  File "asyncore.py", line 390, in handle_read_event
    self.handle_read()
  File "asynchat.py", line 137, in handle_read
    self.found_terminator()
  File "/usr/lib/python2.5/site-packages/pyftpdlib/ftpserver.py", line
1134, in found_terminator
    method(arg) # call
  File "/usr/lib/python2.5/site-packages/pyftpdlib/ftpserver.py", line
1409, in ftp_QUIT
    msg_quit = self.authorizer.get_msg_quit(self.username)
  File "/usr/lib/python2.5/site-packages/pyftpdlib/ftpserver.py", line 330,
in get_msg_quit
    return self.user_table[username]['msg_quit']
KeyError: ''


Please use labels and text to provide additional information.

This happens because we ask the authorizer to tell us which is the user's
quitting message while no user is authenticated.

What steps will reproduce the problem?
1. Connecting to FTPd.
2. Sending a lot of STOU requests.

Please use labels and text to provide additional information.
Every time a STOU is received we start an infinite while loop stopped only
when a name unique for the file is found. We should set a maximum number of
tries to create a unique filename (e.g. 99), so that we decrease the
chances of a DoS situation.

A server-side problem that can occur is when a client is trying to access
an FTP server on an internal network protected by a routing device. 
Because a server response from PASV includes an IP address and a port
number, if this IP address corresponds to a private network then the client
will not be able to connect to that private address.

    Client:  PASV
    Server:  227 Entering Passive Mode (10,0,0,1,204,173)

If left unaltered, the client would try to connect to port 52397 on the
private IP address 10.0.0.1 when in reality it should be connecting to the
external IP address of the routing device (e.g. 72.14.221.104).

A practice commonly used by most ftp servers is permitting to configure the
IP address response coming along PASV response request replacing it with
the NAT's IP address. It would be reasonable having the same opportunity in
pyftpdlib.

FTPServer.py, independently if asyncore main polling loop uses select() or
poll(), raises an exception and block the loop when a high number of
simultaneous connections to handle simultaneously is reached. The number of
maximum connections is system-dependent as well as the type of error
raised. [1]

The problem with the base asyncore is that it provides no way to limit the 
number of acceptable connections. A way to limit them should be implemented
into FTPServer.py to avoid possible Denial of Service attacks.

Don't know if it is within the realm of problems that should be dealt with
by base FTPServer.py but since that a lot of FTP servers permit it, I find
it reasonable that FTPServer.py could provide a way to:
- configure/limit the number of maximum acceptable connections.
- (optionally) configure/limit the maximum number of clients which may be
connected from the same source internet address.

[1] A discussion about it can be found on Python SF tracking system where I
submitted a bug-report:
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1685000&group_id=54
70

Comments are apreciated.

According to RFC959 [1] if an argument is provided with STAT FTP server
should return a directory LISTing over the command channel.

[1] http://www.faqs.org/rfcs/rfc959.html (chapter 4.1.3)

What steps will reproduce the problem?
Using STAT when user is not authenticated yet.

What is the expected output? What do you see instead?
STAT request is rejected with a "530 Log in with USER and PASS first" response.

Please use labels and text to provide additional information.
It would be better accepting STAT even if user isn't authenticated yet.
At the same time we should reject argumented STAT requests since that we
still don't want to provide directory LISTing over the command channel.

Platform:
python 2.5.1
OS X 10.4.1 (Intel)

Steps to reproduce:
1. install SVN revision 37
2. execute test/test_ftpd.py

Error output:
....................FEFE..
======================================================================
ERROR: test_rest_on_stor (__main__.ftp_store_data)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test/test_ftpd.py", line 419, in test_rest_on_stor
    ftp.storbinary('appe 1.tmp', f1)
  File
"/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/ftplib.py",
line 426, in storbinary
    conn = self.transfercmd(cmd)
  File
"/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/ftplib.py",
line 356, in transfercmd
    return self.ntransfercmd(cmd, rest)[0]
  File
"/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/ftplib.py",
line 327, in ntransfercmd
    resp = self.sendcmd(cmd)
  File
"/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/ftplib.py",
line 241, in sendcmd
    return self.getresp()
  File
"/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/ftplib.py",
line 207, in getresp
    resp = self.getmultiline()
  File
"/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/ftplib.py",
line 193, in getmultiline
    line = self.getline()
  File
"/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/ftplib.py",
line 183, in getline
    if not line: raise EOFError
EOFError

There is a typo on line 889, it should be 'password' instead of 'passowrd'.

According to RFC3659 [1] SIZE command request should be rejected if path
name supplied as argument is not a file (e.g. a directory).
Even if this is not expressly declared, all FTPd implementation I've seen
systematically act like this.

[1] http://www.faqs.org/rfcs/rfc3659.html (chapter 4)

A client connected to ftpd sending a STAT command will receive the
following response:

211-Python FTP server library (pyftpdlib) 0.2.0 status:
  Connected to: 127.0.0.1:21
  Logged in as: anonymous
  TYPE: ASCII; STRUcture: File; MODE: Stream
  Data connection closed.
211 End of status.

The current pyftpdlib implementation provides no easy way to modify the
server ident ("pyftpdlib 0.2.0" in this case) provided with STAT response.
It would be good adding a new global variable for users who want to modify it.

What steps will reproduce the problem?
1. Configuring pyftpdlib by setting maximum connections limit option using
Python 2.3:

>>> from pyftpdlib import ftpserver
>>> ftpd = ftpserver.FTPServer(('0.0.0.0', 21), ftpserver.FTPHandler)
>>> ftpd.max_cons = 256
>>> ftpd.serve_forever()
Serving FTP on 0.0.0.0:21

What is the expected output? What do you see instead?
When trying to connect from client the following exception is raised by
pyftpdlib:

Traceback (most recent call last):
  File "C:\Python23\lib\asyncore.py", line 69, in read
    obj.handle_read_event()
  File "C:\Python23\lib\asyncore.py", line 384, in handle_read_event
    self.handle_accept()
  File "C:\Python23\lib\site-packages\pyftpdlib\ftpserver.py", line 2317,
in han
dle_accept
    print self._map
  File "C:\Python23\lib\asyncore.py", line 365, in __getattr__
    return getattr(self.socket, attr)
AttributeError: '_socketobject' object has no attribute '_map'

Please use labels and text to provide additional information.
This happens because Python 2.3 provides a version of asyncore.dispatcher
which does not provide the _map attribute (included in Python 2.4 and
higher) which is used in pyftpdlib for getting the number of current
connected clients.
A hack around such lack should be provided to grant backward compatibility
with Python version prior to 2.4.

According to RFC 959 [1] a REIN command should flush all I/O and account
information, except to allow any transfer in progress to be completed.  All
parameters must be resetted to the default settings and the control
connection should be left open.  This is identical to the state in which a
user finds himself immediately after the control connection is opened.

Current pyftpdlib release should also reset "restart_position" and
"current_type" FTPHandler's class attributes, actually not resetted, and,
if no data transfer is in progress, it should close the existing data
channel instance, if any.

[1] http://www.faqs.org/rfcs/rfc959.html (chapter 4.1.1).

When using pyftpdlib behind a NAT (see Issue #39) one big problem still
exists. The passive FTP connections will use ports from 1024 and up, which
means that you must forward all ports 1024-65535 from the NAT to the FTP
server! And you have to allow many (possibly) dangerous ports in your
firewalling rules! Not a good situation. 

To resolve this, it would be reasonable having a new FTPHandler's attribute
(e.g. "passive_ports") to control what ports pyftpdlib will use for its
passive data transfers.

Patch to fix license string in manual and XHTML markup fixes.

What steps will reproduce the problem?
1. install pyftpdlib
2. Run basic_ftp 
3. Try to change directory on a client ftp

What is the expected output? What do you see instead?
The Konqueror ftp client displays directory does not exist

What version of the product are you using? On what operating system?
Verion - pyftpdlib_0.1.1
OS - windows mobile 6
python version - Python-2.5-20061219 (Python CE 
[http://pythonce.sourceforge.net/] )

Please provide any additional information below.

What steps will reproduce the problem?
Attempt loggin in through USER/PASS commands by providing a wrong username.

What is the expected output? What do you see instead?
We expect that "attempted login" counter is increased while it isn't. 

When a PASV command is issued by client a passive data socket is opened for
accepting the incoming data connection.
If FXP "site-to-site" feature is disabled and the resulting connection
comes from a remote IP address which does not match the client's IP address
a 421 "Service not available, closing control connection." is returned
dropping the foreign connection.

This is a wrong response type since RFC 959 specifies that this response
code type may be returned to any command if the service knows it must shut
down (while it doesn't). Moreover both command and data channels *won't* be
closed.

A 425 "Can't open data connection." would be a more appropriate response type.

Method "collect_incoming_data" of "ftp_handler" class should stop buffering
if received lines are too long (possible Denial-of-Service attacks).
Patch:


class ftp_handler(asynchat.async_chat):

    [...]

    def collect_incoming_data(self, data):        
        self.in_buffer = self.in_buffer + data

+        if len(self.in_buffer) > 2048:

+            self.in_buffer = ""


    def found_terminator(self):        
        line = self.in_buffer.strip()

    [...]

When a PORT command is issued by client and connection attempt fails (i.e.
client specified a wrong port) a 500 "Syntax error, command unrecognized"
response is provided while we should respond with a more appropriate 425
"Can't open data connection" instead.

According to RFC959 HELP command should support also an argument field in
which case it should return more specific informational responses for the
command provided as argument.

The current implementation of DTPHandler.initiate_send method doesn't look
at what is specified in ac_out_buffer_size attribute which represents the
buffer size of the outgoing data defaulting to a maximum of 8192 bytes to
send in a single socket.send() call.

Note that this only happens when sending the data by using the file
producer which uses a buffer of 65536 bytes per read().

This represents a problem when using a modified DTPHandler class having
bandwidth throttling capabilities which needs a more precise throughput
when sending the data (we practically can't set a limit smaller than 65536
bytes per seconds).

We inherited this problem from the fixed asynchat module proposed in
Python's issue #1736190 (http://bugs.python.org/issue1736190) we drew on.

I've fixed this in the revision #162 by having used the buffer() builtin
for slicing the data coming from the file handler. The data is now splitted
in pieces with a lenght equal to the value of the ac_out_buffer_size attribute.

According to:
http://cr.yp.to/ftp/type.html
...FTPServer.py TYPE implementation should understand also "TYPE AN" and 
"TYPE L8" commands indicating respectively ASCII and Binary data transfer 
mode types.

STOU command implementation should follow specifications defined into the 
newer RFC1123 [1] which are different from those defined into RFC959 [2].
The 250 positive response indicated into RFC959 has been declared 
incorrect into RFC1123 that requires 125/150 instead.
Moreover RFC1123 wants an additional "FILE: " string to be returned into 
response:

125 FILE: <filename>
150 FILE: <filename>



[1] http://www.faqs.org/rfcs/rfc959.html (chapter 4.1.3)
[2] http://www.faqs.org/rfcs/rfc1123.html (chapter 4.1.2.9)

PATH TRAVERSAL VULNERABILTY

Most ftp filesystem commands are dangerously affected by path traversal.
The reason of this, is poor path filtering in 'normalize' and 'translate' 
methods of class 'abstracted_fs' (lines 1595-1625).

Tests have been conducted on pyftpdlib 0.1.1 on both windows xp and ubuntu 
7.04, revealing the same vulnerability.

Issuing following commands, user root's parent directory will erroneously 
be listed, giving access to forbidden parts of filesystem.

CWD /
LIST ..

Same problem affects commands like STOR and RETR, allowing an attacker to 
retrieve or upload arbitrary system files. This would be only limited by 
rights under which the server is running. For any reason user must be able 
to gain access to those areas.

In order to solve the problem, I've entirely rewritten both vulnerable 
methods.
I think this solution should be robust enough to avoid any path traversal 
issue.

What steps will reproduce the problem?
1. Connecting to FTPd.
2. Logging in by using USER <username>, PASS <password>.
3. Providing PASS <password>.

What is the expected output? What do you see instead?
When PASS <password> is received for the second time we expect some kind of
error response while we get "230 User logged in" again.

Currently, pyftpdlib does not fetch real permissions, owner and group for
files and folders, instead using default fake values. Since the permissions
and owner/group are not applicable in the same fashion on Windows, such
information is currently UNIX only.

The format_list() method of AbstractedFS should be modified to fetch the
owner/group names for each file and the permissions for owner/group/world
to match the standard "ls" output on UNIX platforms. Platforms where the
necessary modules do not exist should fall back on the default values we
use currently.

What steps will reproduce the problem?
1. Sending a REST request followed by an APPE request.

What is the expected output? What do you see instead?
FTPd should watch for APPE preceded by REST, which makes no sense, in which
case it should reject APPE request responding with some kind of error code
(e.g. 550).

In DTPHandler class, when handle_expt or handle_error events occur, 
we just close the connection without providing a response to user
indicating that something went wrong and that transfer is aborted.
When such events occur we should provide a "426 Connection closed; transfer
aborted." instead.

What steps will reproduce the problem?
1. run test suite prior to Revision 40

Running through the test suite on a Linux box to compare with OS X results,
there was a failure in test_stou() due to the filename being calculated as:

        filename = ftp.sendcmd('stou')[4:]

This caused a later call to os.remove to fail while trying to remove the
file "/path/to/my/test/suite/FILE: .0". The response from sendcmd is
actually in the below format:

        #filename comes in as  150 FILE: $filename

so I made the following change to the test suite to correctly remove the
prefix and also make it a little clearer how the filename was being parsed:

        filename = ftp.sendcmd('stou').replace('150 FILE: ', '')

According to RFC 2577 [1] it's a good idea rejecting data connection when a
privileged port (< 1024) is specified in PORT command.
Since that this is just *suggested* I would rencommend the use of a
specific "permit_unprivileged_port" attribute to be added in ftp_handler
class defaulting to False.

[1] http://www.wu-ftpd.org/rfc/rfc2577.html (chapter 3)

What steps will reproduce the problem?
Sending an 'ABOR' command request while there's no transfer in progress.

What is the expected output? What do you see instead?
According to RFC specification we expect a 225 response code while we get 226.

What steps will reproduce the problem?
1. Sending a 'CDUP' command to FTP server when the current working
directory is different from '/'.

What is the expected output? What do you see instead?
According to RFC959 we expect a 200 or 250 response (RFC959 says that code
200 is required but it also says that CDUP uses the same codes as CWD)
while we get 257.

According to RFC 1123 [1] a 554 reply may result from a FTP service command
that follows a REST command in case that the existing file at the
Server-FTP cannot be repositioned as specified in the REST.

[1] http://www.faqs.org/rfcs/rfc1123.html (chapter 4.1.3.4)

What steps will reproduce the problem?
1. Create a symbolic link.
2. Remove the file pointed by the symbolic link.
3. Run pyftpdlib.
4. Issue a LIST command for the directory containing the "broken" symbolic
link.

What is the expected output? What do you see instead?
Client will receive a "550 No such file or directory" error message because
os.stat() raises an exception when trying to follow the "broken" symbolic
link contained in that directory.
Reproducing the problem is quite easy:

root@uds:~# touch foo 
root@uds:~# ln -s foo x 
root@uds:~# rm foo 
root@uds:~# python 
Python 2.5.1 (r251:54863, May  2 2007, 16:56:35) 
[GCC 4.1.2 (Ubuntu 4.1.2-0ubuntu4)] on linux2 
Type "help", "copyright", "credits" or "license" for more information. 
>>> import os 
>>> os.stat('x') 
Traceback (most recent call last): 
  File "<stdin>", line 1, in <module> 
OSError: [Errno 2] No such file or directory: 'x' 

This is not reasonable since for a single "broken" symbolic link we lose
the remaining listing of all other files in that directory.
To avoid such problem my proposal is using use os.lstat() when os.stat() fails:

>>> os.lstat('x') 
(41471, 425170L, 2049L, 1, 0, 0, 3L, 1193652432, 1193651349, 
1193651349)

Description:

FXP is part of the name of a popular Windows FTP client:
http://www.flashfxp.com. This client has made the name "FXP" commonly used
as a synonym for site-to-site FTP transfers, for transferring a file
between two remote FTP servers without the transfer going through the
client's host. Sometimes "FXP" is referred to as a protocol; in fact, it is
not. The site-to-site transfer capability was deliberately designed into
RFC 959. More info can be found here:
http://www.proftpd.org/docs/howto/FXP.html.


Implementation:

Actually pyftpdlib does not allow site-to-site transfers.
Because of a type of attack known as the "FTP bounce" attack:

http://www.cert.org/advisories/CA-1997-27.html

...it will reject any connection from/to remote IP addresses which do not
match the client's IP address.

However, some site administrators may want to allow their servers to
support site-to-site transfers. 
pyftpdlib should provide a variable to allow administrators to disable
bounce protection for cases where FXP site-to-site transfer capability is
desired.

Currently, the HELP command provided with no arguments lists all the
recognized commands and their description:

        ABOR  abort data-channel transfer
        ALLO  allocate space for file about to be sent (obsolete)
        APPE  resume upload
        CDUP  go to parent directory
        CWD   change current directory
        ...

Most FTPd implementations provide a simplified command listing with only
the command names: 

  ABOR    ALLO    APPE    CDUP    CWD     DELE    HELP    LIST
  MDTM    MKD     MODE    NLST    NOOP    PASS    PASV    PORT
  PWD     QUIT    REIN    REST    RETR    RMD     RNFR    RNTO
  ...

This would have our implementation match the formatting commonly in use by
most FTP servers, and will be easier for the client to parse.

We actually lack documentation for the module.
I was thinking of writing it by using Wiki service provided with Google Code.
Alternatively it would be also good using LATEX format complying ourself
with the standard Python doc.

What steps will reproduce the problem?
1. Starting FTPd.
2. Connecting one or more client(s).
3. Stopping FTPd via KeyboardInterrupt or SIGTERM.

What is the expected output? What do you see instead?
A memory leak occurs. It may be detected by using:
>>> import gc
>>> gc.set_debug(gc.DEBUG_LEAK)

Please use labels and text to provide additional information.
When FTPd is stopped via KeyboardInterrupt or SIGTERM we should first close
all existent channels before definitively exit.

According to RFC959 [1] a new USER command could be entered at any point 
in order to change the access control flushing any user, password, and 
account information already supplied and beginning the login sequence 
again. 

[1] http://www.faqs.org/rfcs/rfc959.html (chapter 4.1.1).

The logic used to find out a unique file name when processing STOU command
is unsafe and should not be used.
The fix proposed by me in Issue 25 (SVN revision #37) consist of first
finding out a unique file name by using os.path.exists(), and then by
creating the new file with open() builtin function.
A similar approach is also used in the current 0.1.1 pyftpdlib release.

This methodology is unsafe since the file name refers to a file that did
not exist at some point, but by the time you get around to creating it,
someone else may have beaten you to the punch.

I propose to improve command help strings by providing command's syntax
other than just its description (e.g. 'APPE' : "Syntax APPE <SP> file-name
(append data to an existent file)").
A lot of other ftpd implementations does that and it would be reasonable
having pyftpdlib doing the same. 

The ABOR command is not currently tested in the unit test implementation.
The unit tests should be updated to include testing for various ABOR use
cases: 

Case 1: ABOR while no data channel is opened: respond with 225.

Case 2: user sends a PASV, a data-channel socket is listening but not
connected, and ABOR is sent: close listening data socket, respond with 225.

Case 3: data channel opened with PASV or PORT, but ABOR sent before a data
transfer has been started: close data channel, respond with 225.

Case 4: ABOR while a data transfer on DTP channel is in progress:
close data channel, respond with 426, respond with 226.

By using Python versions prior to 2.4 pyftpdlib fails starting on systems
where poll() is not available.

<snippet>
Traceback (most recent call last):
   File "/ffs/lib/python2.3/site-packages/pyftpdlib/FTPServer.py", line
1206, in serve_forever
    asyncore.loop(timeout=1, use_poll=True)
  File "/tgtsvr/lib/python2.3/asyncore.py", line 193, in loop
  File "/tgtsvr/lib/python2.3/asyncore.py", line 128, in poll2
ImportError: No module named poll
</snippet>

This does not happen with newer asyncore versions (included with Python 2.4
and higher) which first check if poll() is available or not, in which case
select() is silently used.
Instead of telling asyncore to use poll by default we should first check if
poll is available or not by simply using the following statement:

>>> asyncore.loop(timeout=1, use_poll=hasattr(asyncore.select, 'poll'))

This should grant backward compatibility with older asyncore versions
included with Python 2.2, 2.3 and 2.4.


Thanks to Greg Copeland (GCopeland AT efjohnson DOT com) for having
submitted such report.

When a HELP command is issued with an argument ftpd must return the help of
that particular command.
If such command is not valid (i.e. unknown) the actual pyftpdlib
implementation respond with 500 "Syntax error, command unrecognized" while
it would be more appropriate responding with 501 "Syntax error in
parameters or arguments" or 502 "Command not implemented".