-
-
Notifications
You must be signed in to change notification settings - Fork 201
[Bug] FTP server disconnects and freezes #760
Description
There seem to be issues with the current FTP server implementation existing for several years now, so I'm creating this thread to help with efforts to find and fix them. Most of them happen when transferring large amounts of files or using less widespread clients or libraries, such as Rclone, that enforce the protocol strictly.
From my observations, they can be categorized these groups:
- Server sends an invalid response, forcing the client to reconnect or ignore the unexpected response
- Server stops responding in the current connection, leading to timeouts and reconnects on client side
- Server closes the connection unexpectedly, forcing the client to reconnect or abort the operation
- Server stops receiving any connections, making it impossible to make a new connection
Looking into and debugging the code, I found 4 bugs so far:
Bug 1 - When entering passive mode, server never responds to the new connection
This happens when slisten() function fails to bind socket to requested port, but returns a valid socket handle anyways. This can happen for multiple reasons, such as the port being already used by another thread. Server then tells client to connect to an invalid port, resulting in timeout and disconnect.
Solution: Return error in slisten() when bind() or listen() fails.
Bug 2 - After entering passive mode and accepting connection, server stops responding
This sneaky one happens when accept() returns a socket handle with ID 0 (valid range is 0 - 1023), as the code incorrectly treats that as an invalid handle. Client eventually times out, but the server side thread seems to never exit, resulting in a thread buildup and server rejecting new connections. May even happen more often than every 1024 files, as WMM isn't the only one creating new sockets.
Solution: Take 0 as a valid socket, turning > 0 into >= 0.
Bug 3 - CWDing into non-existing directory returns error for a path never requested
For example:
Request: CWD /dev_hdd0/packages/aaa/bbb
Response: 550 File Error "///dev_hdd0/packages/aaa/bbb"
Request: CWD /dev_hdd0/packages/aaa/New folder
Response: 550 File Error "/dev_hdd0/packages/aaa/bbb//dev_hdd0/packages/aaa/New folder"
findPath() seems to treat absolute paths as relative sometimes, leading to concatenation of current working directory with the requested one. Some clients close connection after receiving this kind of response, in which case it's unusable.
Solution: findPath() should only do that when the requested path is relative and keep the original path intact otherwise
Also to note: sprintf() used for concatenation may overflow beyond the buffer limit and corrupt the stack contents, use of snprintf() with the correct buffer size would be better
Bug 4 - Under heavy FTP use, OS kills all connections with FIN packets, unexpectedly
This one is really annoying to debug since it also affects every network connection used sending logs and telemetry data. The only correlation I found is that it can be replicated with Rclone (only passive mode supported) sending large amounts of small files quickly, thus creating and closing sockets at a fast pace. Strangely enough, it always seems to die after 159th file and ~25 seconds since the start of the transfer (note: connections made by debugging may affect those values, but it's consistent for each version of source code). Filezilla set to forced active mode seems to bypass this issue. Logging into files on device may be required to trace what's going on.
Not-really a solution: Limitting files per second for passive mode under the magic value does bypass this issue, but it's impossible to calculate it on-the-fly.
Will post a pull request with fixes for bugs 1, 2 and 3 shortly.