[VIM] FileZilla DoS issues - questions, answers, more questions

[Steven M. Christey]

Sources:

1)  rgod http://retrogod.altervista.org/filezilla_0921_dos.html

    DoS apparently using STOR with ".." sequences in the argument.

2) shinnai http://www.milw0rm.com/exploits/2914

   DoS apparently using LIST/NLST with just "A*"

Various DBs are linking these to:

  http://sourceforge.net/project/shownotes.php?release_id=470364&group_id=21558

and claiming them as null derefs.

The FileZilla 0.9.22 changelog: "Release Name: 0.9.22 Notes: Update
hightly recommended due to fixed denial of service vulnerability.
Changes: Fixed bugs: - Fix denial of service vulnerability due to
nullpointer dereference. - Added support for broken clients sending
CWD command without arguments."



Initial Uncertainties
---------------------

1) I can't find any clear relationship between the rgod/shinnai
   disclosures and the FileZilla fix, except the fix came out around
   the same day as the disclosures.  But all we know better than to
   think of it as proof, right?

2) It seems pretty weird that "LIST A*" would trigger a bug, since
   wildcard functionality bugs would be caught right away.  And if so,
   why would it trigger a null deref?  (or "NLST A*")

3) What do STOR and LIST/NLST have in common?  You need to specify an
   alternate channel for the data transfer.  And notice that both
   exploits share the following code:

    $in = "PASV ".$junk."\r\n";
    socket_write($socket, $in, strlen ($in));

    $in = "PORT ".$junk."\r\n";
    socket_write($socket, $in, strlen ($in));

  This will produce an invalid PORT command for both exploits,
  according to RFC 959.  (The PASV command isn't supposed to have any
  args at all, but maybe that's not relevant here.)

4) So, maybe the problem isn't in the STOR command or LIST commands,
   but rather in how the malformed port is being handled.

Seeming Revelations
-------------------

5) CVS logs include a couple changes involving "fix null pointer
   dereference", by defining and using a function
   "CPermissions::DestroyDirlisting".

    http://filezilla.cvs.sourceforge.net/filezilla/FileZilla%20Server/source/Permissions.cpp?r1=1.74&view=log

6) Relevant change in source/TransferSocket.cpp:

  http://filezilla.cvs.sourceforge.net/filezilla/FileZilla%20Server/source/TransferSocket.cpp?r1=1.55&r2=1.56

  This just refactors the code to use the new
  CPermissions::DestroyDirlisting.

7) The best changes are here:

  http://filezilla.cvs.sourceforge.net/filezilla/FileZilla%20Server/source/ControlSocket.cpp?r1=1.129&r2=1.130

  http://filezilla.cvs.sourceforge.net/filezilla/FileZilla%20Server/source/ControlSocket.cpp?r1=1.129&r2=1.130

  You can see how it's resetting a ".pasv" value, and in some places
  also calls CPermissions::DestroyDirlisting and a break if there's no
  socket... which it didn't do previously.  Lines 1054, 1767 emphasize
  this.  Each of these changes occur within a check for
  "(!m_transferstatus.socket)".  Before this code was inserted, a null
  dereference probably would have happened in the next few lines of
  code, since m_transferstatus.socket is assumed to be non-null.

  This is the kind of behavior you'd expect from a malformed PORT
  command, because no socket would be created.

  DISCLAIMER: I'm not *sure* of this, but lookie here:

http://filezilla.cvs.sourceforge.net/filezilla/FileZilla%20Server/source/ControlSocket.cpp?annotate=1.130#l757

  736 : botg 1.32	case COMMAND_PORT:
  ...
  757 : botg 1.120			if (count != 5)
  758 : botg 1.11						  {
  759 : botg 1.122								Send(_T("501 Syntax error"));
***  760 : botg 1.130				m_transferstatus.pasv = -1;
***  761 : botg 1.11												break;
  762 :															}

  

Note that "***" is the newly added line.

In the COMMAND_STOR code, we have:

  1231 : botg 1.120			 if (m_transferstatus.pasv == -1)
  1232 : botg 1.1										   {
  1233 : botg 1.122                 Send(_T("503 Bad sequence of commands."));
  1234 : botg 1.1					break;
  1235 :								}


So, in older versions, m_transferstatus.pasv would NOT have been -1 if
the PORT command was invalid, so later code would have executed that
involved sending data over a socket that doesn't exist.


8) So, given the relevant changes, we can at least assume that the
   vendor fixed the reported bugs.


Remaining Questions
-------------------

9) So, what of the STOR and LIST/NLST issues themselves?  Do they have
   any problems at all, or is it just the PORT command?

   Answer: I don't know.  This code is spaghetti, the diffs are
   extensive, and my head hurts.  To start, you might want to see how
   the STOR code handles a non-existent file, and whether it can
   handle LIST/NLST with wildcards that don't return any matches at
   all.


- Steve