From 635fad7768f79be3dd2755f98c5a79db44daa600 Mon Sep 17 00:00:00 2001
From: Rob Swindell <rob@synchro.net>
Date: Mon, 15 Feb 2021 14:13:21 -0800
Subject: [PATCH] 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 | 8 ++++----
src/sbbs3/ctrl/FtpCfgDlgUnit.dfm | 6 +++---
src/sbbs3/ctrl/FtpCfgDlgUnit.h | 2 +-
src/sbbs3/ftpsrvr.c | 6 ++++--
src/sbbs3/ftpsrvr.h | 2 ++
5 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/src/sbbs3/ctrl/FtpCfgDlgUnit.cpp b/src/sbbs3/ctrl/FtpCfgDlgUnit.cpp
index 4b6003e72c..62ff5fcafc 100644
--- a/src/sbbs3/ctrl/FtpCfgDlgUnit.cpp
+++ b/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
diff --git a/src/sbbs3/ctrl/FtpCfgDlgUnit.dfm b/src/sbbs3/ctrl/FtpCfgDlgUnit.dfm
index 82a9f1f7e0..3164e24070 100644
--- a/src/sbbs3/ctrl/FtpCfgDlgUnit.dfm
+++ b/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
diff --git a/src/sbbs3/ctrl/FtpCfgDlgUnit.h b/src/sbbs3/ctrl/FtpCfgDlgUnit.h
index 1c40de6605..932a2d7ac5 100644
--- a/src/sbbs3/ctrl/FtpCfgDlgUnit.h
+++ b/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;
diff --git a/src/sbbs3/ftpsrvr.c b/src/sbbs3/ftpsrvr.c
index c14597fa53..f10c1de8d6 100644
--- a/src/sbbs3/ftpsrvr.c
+++ b/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);
diff --git a/src/sbbs3/ftpsrvr.h b/src/sbbs3/ftpsrvr.h
index d9a0a2d1d0..028e57dc7f 100644
--- a/src/sbbs3/ftpsrvr.h
+++ b/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" },
--
GitLab