cathery / sys-ftpd

Nintendo Switch FTP server as a background service (sysmodule)
GNU General Public License v3.0
240 stars 14 forks source link

Fix broken authentication #11

Closed bratkartoffel closed 4 years ago

bratkartoffel commented 4 years ago

This commit fixes multiple issues with the authentication mechanism:

cathery commented 4 years ago

I appreciate the PR. Most of it seems good to me. "Broken" is a strong word, as the authentication system seems to work just fine. Instead I'm assuming these are security vulnerabilites.

Different message for wrong username and wrong password

I can understand the change behind this. This is fine by me.

Authentication state was never checked

I don't know what this means. Can you give me an example of how this can be abused? I also noticed that you've added the checks for some commands, but not for others. Is that intentional?

bratkartoffel commented 4 years ago

The problem is, that a malicious client can simply skip authentication and upload / download files without authentication.

My configuration:

[User]
user:=foo
[Password]
password:=bar
[Port]
port:=5000
[Anonymous]
anonymous:=0

You can easily reproduce this issue by using telnet or nc:

[07:40] root@server:~# telnet 192.168.2.141 5000
Trying 192.168.2.141...
Connected to 192.168.2.141.
Escape character is '^]'.
220 Hello!
PWD
257 "/"
MKD foobar
250 OK
PASV
227 Entering Passive Mode (192,168,2,141,235,202)

image

I've added the authentication checks only for functions that need authentication (I think). So the informational commands (like HELP or FEAT) still work without authentication.

cathery commented 4 years ago

Thank you for telling me how to reproduce the issue. I understand it now