Disable FTP Bounce (FXP) support by default

The Synchronet FTP server has (since 2001) disallowed PORT/EPRT/LPRT commands with a "reserved" port number (i.e. < 1024) as recommended by RFC2577 and when attempted, would log a "SUSPECTED FTP BOUNCE HACK ATTEMPT" in the data/hack.log file.

However, as Karloch (HISPAMSX) pointed out recently, an FTP Bounce Attack to other TCP ports was still possible (and detected/reported by some security scans as a potential vulnerability).

So, reject all PORT/EPRT/LPRT commands that specify an IP address other than that used for the control TCP connection unless the sysop specifically enables the new "ALLOW_BOUNCE" option flag (in the [ftp] section of sbbs.ini) and the user is an authenticated non-guest/anonymous user. And as before, log the attempt as a suspected hack attempt.

This change also removes the "Directory File Access" checkbox from the Synchronet Control Panel for Windows as that feature is "going away" soon (or at least, it won't be an FTP-specific option/feature if it remains).

src/sbbs3/ctrl/FtpCfgDlgUnit.cpp

@@ -78,7 +78,7 @@ void __fastcall TFtpCfgDlg::FormShow(TObject *Sender)
     CmdLogCheckBox->Checked=MainForm->ftp_startup.options&FTP_OPT_DEBUG_RX;
 	DebugTxCheckBox->Checked=MainForm->ftp_startup.options&FTP_OPT_DEBUG_TX;
 	DebugDataCheckBox->Checked=MainForm->ftp_startup.options&FTP_OPT_DEBUG_DATA;
-    DirFilesCheckBox->Checked=MainForm->ftp_startup.options&FTP_OPT_DIR_FILES;
+    AllowBounceCheckBox->Checked=MainForm->ftp_startup.options&FTP_OPT_ALLOW_BOUNCE;
     AllowQWKCheckBox->Checked
         =MainForm->ftp_startup.options&FTP_OPT_ALLOW_QWK;
     LocalFileSysCheckBox->Checked
@@ -162,10 +162,10 @@ void __fastcall TFtpCfgDlg::OKBtnClick(TObject *Sender)
     	MainForm->ftp_startup.options|=FTP_OPT_ALLOW_QWK;
     else
 	    MainForm->ftp_startup.options&=~FTP_OPT_ALLOW_QWK;
-	if(DirFilesCheckBox->Checked==true)
-    	MainForm->ftp_startup.options|=FTP_OPT_DIR_FILES;
+	if(AllowBounceCheckBox->Checked==true)
+    	MainForm->ftp_startup.options|=FTP_OPT_ALLOW_BOUNCE;
     else
-	    MainForm->ftp_startup.options&=~FTP_OPT_DIR_FILES;
+	    MainForm->ftp_startup.options&=~FTP_OPT_ALLOW_BOUNCE;
 	if(LocalFileSysCheckBox->Checked==false)
     	MainForm->ftp_startup.options|=FTP_OPT_NO_LOCAL_FSYS;
     else

src/sbbs3/ctrl/FtpCfgDlgUnit.dfm

@@ -164,13 +164,13 @@ object FtpCfgDlg: TFtpCfgDlg
         ShowHint = True
         TabOrder = 1
       end
-      object DirFilesCheckBox: TCheckBox
+      object AllowBounceCheckBox: TCheckBox
         Left = 148
         Top = 86
         Width = 125
         Height = 20
-        Hint = 'Allow users access to files in directory, but not in database'
-        Caption = 'Directory File Access'
+        Hint = 'Allow authenticated users to use FTP Bounce (not recommended)'
+        Caption = 'Allow Bouncing'
         ParentShowHint = False
         ShowHint = True
         TabOrder = 8

src/sbbs3/ctrl/FtpCfgDlgUnit.h

@@ -69,7 +69,7 @@ __published:
     TButton *OKBtn;
     TButton *CancelBtn;
     TButton *ApplyBtn;
-    TCheckBox *DirFilesCheckBox;
+    TCheckBox *AllowBounceCheckBox;
     TTabSheet *IndexTabSheet;
     TCheckBox *AutoIndexCheckBox;
     TEdit *IndexFileNameEdit;

src/sbbs3/ftpsrvr.c

@@ -2873,7 +2873,7 @@ static void ctrl_thread(void* arg)
 			if(pasv_sock!=INVALID_SOCKET)  {
 				ftp_close_socket(&pasv_sock,&pasv_sess,__LINE__);
 			}
-
+			memcpy(&data_addr, &ftp.client_addr, ftp.client_addr_len);
 			p=cmd+5;
 			SKIP_WHITESPACE(p);
 			if(strnicmp(cmd, "PORT ",5)==0) {
@@ -3009,7 +3009,9 @@ static void ctrl_thread(void* arg)
 			}
 
 			inet_addrtop(&data_addr, data_ip, sizeof(data_ip));
-			if(data_port< IPPORT_RESERVED) {
+			bool bounce_allowed = (startup->options & FTP_OPT_ALLOW_BOUNCE) && !(user.rest & FLAG('G'));
+			if(data_port < IPPORT_RESERVED
+				|| (memcmp(&data_addr, &ftp.client_addr, ftp.client_addr_len) != 0 && !bounce_allowed)) {
 				lprintf(LOG_WARNING,"%04d <%s> !SUSPECTED BOUNCE ATTACK ATTEMPT to %s port %u"
 					,sock,user.alias
 					,data_ip,data_port);

src/sbbs3/ftpsrvr.h

@@ -106,6 +106,7 @@ static struct init_field ftp_init_fields[] = {
 #define FTP_OPT_NO_LOCAL_FSYS		(1<<5)
 #define FTP_OPT_DIR_FILES			(1<<6)	/* Allow access to files in dir but not in database */
 #define FTP_OPT_KEEP_TEMP_FILES		(1<<7)	/* Don't delete temp files (for debugging) */
+#define FTP_OPT_ALLOW_BOUNCE		(1<<8)
 #define FTP_OPT_LOOKUP_PASV_IP		(1<<9)	/* resolve public IP address for PASV response */
 #define FTP_OPT_NO_HOST_LOOKUP		(1<<11)
 #define FTP_OPT_NO_RECYCLE			(1<<27)	/* Disable recycling of server		*/
@@ -125,6 +126,7 @@ static ini_bitdesc_t ftp_options[] = {
 	{ FTP_OPT_NO_LOCAL_FSYS			,"NO_LOCAL_FSYS"		},
 	{ FTP_OPT_DIR_FILES				,"DIR_FILES"			},
 	{ FTP_OPT_KEEP_TEMP_FILES		,"KEEP_TEMP_FILES"		},
+	{ FTP_OPT_ALLOW_BOUNCE			,"ALLOW_BOUNCE"			},
 	{ FTP_OPT_LOOKUP_PASV_IP		,"LOOKUP_PASV_IP"		},
 	{ FTP_OPT_NO_HOST_LOOKUP		,"NO_HOST_LOOKUP"		},
 	{ FTP_OPT_NO_RECYCLE			,"NO_RECYCLE"			},