hfiref0x / LightFTP

Small x86-32/x64 FTP Server
BSD 2-Clause "Simplified" License
232 stars 83 forks source link

Privileged port numbers being used for PASV port connection on little endian machines #4

Closed ve3jrw closed 7 years ago

ve3jrw commented 7 years ago

The bind function requires that the port number be specified in "network byte order" which is big endian.

This is not accounted for on little endian machines in the code and results in low port numbers being used and transfer failures.

PASV ports must be > 1024, outside of the privileged port range.

In one test we found the network PASV port being setup was 96. This was due to the use of the little endian value 0x6000 intended to be port 24576 but ends up as port 96 (0x0060) with bind because of the big endian requirement. This is in the privileged port range and results in a transfer failure.

The fix is to byte swap the port value for the bind structure on Windows and other little endian machines.

hfiref0x commented 7 years ago

Hello! Thank you for your feedback, but your case seems weird. As you can see in source code, all port number calculations in PASV take place in native byte order to the host system, but before sent to a sockaddr structure resulting value is converted by the htons function. htons is intended to convert from host native endianess to network byte order. If one is not converted, then server will not work at all in passive mode due to PASV will send wrong port number to the client. Please take it into account while debugging.

ve3jrw commented 7 years ago

On 6/3/2017 3:07 AM, hfiref0x wrote:

Hello! Thank you for you feedback, but your case seems weird. As you can see in source code, all port number calculations in PASV take place in native byte order to the host system, but before sent to a sockaddr structure resulting value is converted by the htons function. htons is intended to convert from host native endianess to network byte order. If one is not converted, then server will not work at all in passive mode due to PASV will send wrong port number to the client. Please take it into account while debugging.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/hfiref0x/LightFTP/issues/4#issuecomment-305957141, or mute the thread https://github.com/notifications/unsubscribe-auth/AWkHn2MIMxwIS_KDN7ozsZF2AMbo9Iu9ks5sAQY_gaJpZM4NucDQ.

This is exactly what is happening. PASV is sending the wrong port number to the client when it gets to the privileged range.

Your call, it is a real bug, easy to replicate, I've fixed it in our code copy and it solved the problems we were experiencing. Just thought I would pass it along to you, but you can ignore it if you want.

hfiref0x commented 7 years ago

Hello,

you can always do a pull request or simple post here your changes.

ve3jrw commented 7 years ago

We picked up the code base in the February time frame and changed it into a DLL so we can't pull with your code. Also we had fixed a couple of other problems.

Looking at your latest code base, this bug has been corrected in the April 25th update as the prior code didn't use htons to setup the correct endian order. Shouldn't be a problem any longer.