Bug 958: Local ports for active FTP mode can be limited

https://winscp.net/tracker/958

Source commit: ed7c78a9a307a91b50f373686f882f4b82386e3b

source/core/Configuration.cpp

@@ -111,6 +111,8 @@ void __fastcall TConfiguration::Default()
   FCacheDirectoryChangesMaxSize = 100;
   FShowFtpWelcomeMessage = false;
   FExternalIpAddress = L"";
+  FLocalPortNumberMin = 0;
+  FLocalPortNumberMax = 0;
   FTryFtpWhenSshFails = true;
   FParallelDurationThreshold = 10;
   FMimeTypes = UnicodeString();
@@ -244,6 +246,8 @@ UnicodeString __fastcall TConfiguration::PropertyToKey(const UnicodeString & Pro
     KEY(Integer,  CacheDirectoryChangesMaxSize); \
     KEY(Bool,     ShowFtpWelcomeMessage); \
     KEY(String,   ExternalIpAddress); \
+    KEY(Integer,  LocalPortNumberMin); \
+    KEY(Integer,  LocalPortNumberMax); \
     KEY(Bool,     TryFtpWhenSshFails); \
     KEY(Integer,  ParallelDurationThreshold); \
     KEY(String,   MimeTypes); \
@@ -1597,6 +1601,21 @@ void __fastcall TConfiguration::SetExternalIpAddress(UnicodeString value)
   SET_CONFIG_PROPERTY(ExternalIpAddress);
 }
 //---------------------------------------------------------------------
+bool TConfiguration::HasLocalPortNumberLimits()
+{
+  return (LocalPortNumberMin > 0) && (LocalPortNumberMax >= LocalPortNumberMin);
+}
+//---------------------------------------------------------------------
+void TConfiguration::SetLocalPortNumberMin(int value)
+{
+  SET_CONFIG_PROPERTY(LocalPortNumberMin);
+}
+//---------------------------------------------------------------------
+void TConfiguration::SetLocalPortNumberMax(int value)
+{
+  SET_CONFIG_PROPERTY(LocalPortNumberMax);
+}
+//---------------------------------------------------------------------
 void __fastcall TConfiguration::SetMimeTypes(UnicodeString value)
 {
   SET_CONFIG_PROPERTY(MimeTypes);

source/core/Configuration.h

@@ -71,6 +71,8 @@ private:
   UnicodeString FRandomSeedFile;
   UnicodeString FPuttyRegistryStorageKey;
   UnicodeString FExternalIpAddress;
+  int FLocalPortNumberMin;
+  int FLocalPortNumberMax;
   bool FTryFtpWhenSshFails;
   int FParallelDurationThreshold;
   bool FScripting;
@@ -146,6 +148,8 @@ private:
   void __fastcall SetCollectUsage(bool value);
   bool __fastcall GetIsUnofficial();
   bool __fastcall GetPersistent();
+  void SetLocalPortNumberMin(int value);
+  void SetLocalPortNumberMax(int value);
 
 protected:
   TStorage FStorage;
@@ -260,6 +264,7 @@ public:
   UnicodeString __fastcall GetFileVersion(const UnicodeString & FileName);
   UnicodeString __fastcall GetFileMimeType(const UnicodeString & FileName);
   bool RegistryPathExists(const UnicodeString & RegistryPath);
+  bool HasLocalPortNumberLimits();
 
   TStoredSessionList * __fastcall SelectFilezillaSessionsForImport(
     TStoredSessionList * Sessions, UnicodeString & Error);
@@ -318,6 +323,8 @@ public:
   __property int CacheDirectoryChangesMaxSize = { read = FCacheDirectoryChangesMaxSize, write = SetCacheDirectoryChangesMaxSize };
   __property bool ShowFtpWelcomeMessage = { read = FShowFtpWelcomeMessage, write = SetShowFtpWelcomeMessage };
   __property UnicodeString ExternalIpAddress = { read = FExternalIpAddress, write = SetExternalIpAddress };
+  __property int LocalPortNumberMin = { read = FLocalPortNumberMin, write = SetLocalPortNumberMin };
+  __property int LocalPortNumberMax = { read = FLocalPortNumberMax, write = SetLocalPortNumberMax };
   __property bool TryFtpWhenSshFails = { read = FTryFtpWhenSshFails, write = SetTryFtpWhenSshFails };
   __property int ParallelDurationThreshold = { read = FParallelDurationThreshold, write = SetParallelDurationThreshold };
   __property UnicodeString MimeTypes = { read = FMimeTypes, write = SetMimeTypes };

source/core/FtpFileSystem.cpp

@@ -2667,14 +2667,15 @@ int __fastcall TFTPFileSystem::GetOptionVal(int OptionID) const
       break;
 
     case OPTION_LIMITPORTRANGE:
-      Result = FALSE;
+      Result = !FTerminal->SessionData->FtpPasvMode && FTerminal->Configuration->HasLocalPortNumberLimits();
       break;
 
     case OPTION_PORTRANGELOW:
+      Result = FTerminal->Configuration->LocalPortNumberMin;
+      break;
+
     case OPTION_PORTRANGEHIGH:
-      // should never get here OPTION_LIMITPORTRANGE being zero
-      DebugFail();
-      Result = 0;
+      Result = FTerminal->Configuration->LocalPortNumberMax;
       break;
 
     case OPTION_ENABLE_IPV6:

source/core/SessionInfo.cpp

@@ -1413,6 +1413,14 @@ void __fastcall TSessionLog::DoAddStartupInfo(TSessionData * Data)
         (BooleanToEngStr(Data->Compression)));
     }
 
+    if ((Data->FSProtocol == fsFTP) && !Data->FtpPasvMode)
+    {
+      if (!Configuration->ExternalIpAddress.IsEmpty() || Configuration->HasLocalPortNumberLimits())
+      {
+        AddSeparator();
+        ADF(L"FTP active mode interface: %s:%d-%d", (DefaultStr(Configuration->ExternalIpAddress, L"<system address>"), Configuration->LocalPortNumberMin, Configuration->LocalPortNumberMax));
+      }
+    }
     AddSeparator();
   }
 }

source/filezilla/AsyncSocketEx.cpp

@@ -796,7 +796,7 @@ BOOL CAsyncSocketEx::Bind(UINT nSocketPort, LPCTSTR lpszSocketAddress)
       return FALSE;
 
     for (res = res0; res; res = res->ai_next)
-      if (Bind(res->ai_addr, res->ai_addrlen))
+      if (BindToAddr(res->ai_addr, res->ai_addrlen))
       {
         ret = TRUE;
         break;
@@ -817,7 +817,7 @@ BOOL CAsyncSocketEx::Bind(UINT nSocketPort, LPCTSTR lpszSocketAddress)
     sockAddr6.sin6_addr = in6addr_any ;
     sockAddr6.sin6_port = htons((u_short)nSocketPort);
 
-    return Bind((SOCKADDR*)&sockAddr6, sizeof(sockAddr6));
+    return BindToAddr((SOCKADDR*)&sockAddr6, sizeof(sockAddr6));
   }
   else if (!lpszAscii && m_SocketData.nFamily == AF_INET)
   {
@@ -828,13 +828,13 @@ BOOL CAsyncSocketEx::Bind(UINT nSocketPort, LPCTSTR lpszSocketAddress)
     sockAddr.sin_addr.s_addr = INADDR_ANY ;
     sockAddr.sin_port = htons((u_short)nSocketPort);
 
-    return Bind((SOCKADDR*)&sockAddr, sizeof(sockAddr));
+    return BindToAddr((SOCKADDR*)&sockAddr, sizeof(sockAddr));
   }
   else
     return FALSE ;
 }
 
-BOOL CAsyncSocketEx::Bind(const SOCKADDR* lpSockAddr, int nSockAddrLen)
+BOOL CAsyncSocketEx::BindToAddr(const SOCKADDR* lpSockAddr, int nSockAddrLen)
 {
   if (!bind(m_SocketData.hSocket, lpSockAddr, nSockAddrLen))
     return TRUE;

source/filezilla/AsyncSocketEx.h

@@ -131,8 +131,8 @@ public:
   BOOL AsyncSelect(long lEvent = FD_READ | FD_WRITE | FD_OOB | FD_ACCEPT | FD_CONNECT | FD_CLOSE);
 
   // Associates a local address with the socket.
-  BOOL Bind(UINT nSocketPort, LPCTSTR lpszSocketAddress);
-  BOOL Bind(const SOCKADDR* lpSockAddr, int nSockAddrLen);
+  virtual BOOL Bind(UINT nSocketPort, LPCTSTR lpszSocketAddress);
+  BOOL BindToAddr(const SOCKADDR* lpSockAddr, int nSockAddrLen);
 
   // Closes the socket.
   virtual void Close();

source/filezilla/TransferSocket.cpp

@@ -913,24 +913,29 @@ BOOL CTransferSocket::Create(BOOL bUseSsl)
   {
     int min=GetOptionVal(OPTION_PORTRANGELOW);
     int max=GetOptionVal(OPTION_PORTRANGEHIGH);
-    if (min>=max)
+    if (min > max)
     {
       m_pOwner->ShowStatus(IDS_ERRORMSG_CANTCREATEDUETOPORTRANGE,FZ_LOG_ERROR);
       return FALSE;
     }
     int startport=static_cast<int>(min+((double)rand()*(max-min))/(RAND_MAX+1));
     int port=startport;
-    while (!CAsyncSocketEx::Create(port, SOCK_STREAM, FD_READ | FD_WRITE | FD_OOB | FD_ACCEPT | FD_CONNECT | FD_CLOSE, 0, GetFamily()))
+    // Failure to create the socket, calls Close(), which resets the family. We want to keep trying the original faimily with each port.
+    // Only with the specific family set, the Create actually does bind(), without which the port testing does not work.
+    int family = GetFamily();
+    DebugAssert(family != AF_UNSPEC);
+    while (!CAsyncSocketEx::Create(port, SOCK_STREAM, FD_READ | FD_WRITE | FD_OOB | FD_ACCEPT | FD_CONNECT | FD_CLOSE, 0, family))
     {
       port++;
       if (port>max)
         port=min;
       if (port==startport)
       {
-        m_pOwner->ShowStatus(IDS_ERRORMSG_CANTCREATEDUETOPORTRANGE,FZ_LOG_ERROR);
+        m_pOwner->ShowStatus(IDS_ERRORMSG_CANTCREATEDUETOPORTRANGE, FZ_LOG_ERROR);
         return FALSE;
       }
     }
+    LogMessage(FZ_LOG_INFO, L"Selected port %d", port);
   }
 
   return TRUE;

source/forms/Preferences.cpp

@@ -33,6 +33,8 @@
 #pragma link "PathLabel"
 #pragma resource "*.dfm"
 //---------------------------------------------------------------------
+const int PrivatePortMin = 49152; // https://en.wikipedia.org/wiki/List_of_TCP_and_UDP_port_numbers#Dynamic,_private_or_ephemeral_ports
+//---------------------------------------------------------------------
 bool __fastcall DoPreferencesDialog(TPreferencesMode APreferencesMode,
   TPreferencesDialogData * DialogData)
 {
@@ -628,6 +630,11 @@ void __fastcall TPreferencesDialog::LoadConfiguration()
     RetrieveExternalIpAddressButton->Checked = Configuration->ExternalIpAddress.IsEmpty();
     CustomExternalIpAddressButton->Checked = !RetrieveExternalIpAddressButton->Checked;
     CustomExternalIpAddressEdit->Text = Configuration->ExternalIpAddress;
+    LocalPortNumberCheck->Checked = Configuration->HasLocalPortNumberLimits();
+    LocalPortNumberMinEdit->AsInteger =
+      (Configuration->LocalPortNumberMin > 0 ? Configuration->LocalPortNumberMin : PrivatePortMin);
+    LocalPortNumberMaxEdit->AsInteger =
+      (Configuration->LocalPortNumberMax >= LocalPortNumberMinEdit->AsInteger ? Configuration->LocalPortNumberMax : static_cast<int>(LocalPortNumberMaxEdit->MaxValue));
     TryFtpWhenSshFailsCheck->Checked = Configuration->TryFtpWhenSshFails;
 
     // logging
@@ -960,6 +967,8 @@ void __fastcall TPreferencesDialog::SaveConfiguration()
     // network
     Configuration->ExternalIpAddress =
       (CustomExternalIpAddressButton->Checked ? CustomExternalIpAddressEdit->Text : UnicodeString());
+    Configuration->LocalPortNumberMin = LocalPortNumberCheck->Checked ? LocalPortNumberMinEdit->AsInteger : 0;
+    Configuration->LocalPortNumberMax = LocalPortNumberCheck->Checked ? LocalPortNumberMaxEdit->AsInteger : 0;
     Configuration->TryFtpWhenSshFails = TryFtpWhenSshFailsCheck->Checked;
 
     // security
@@ -1394,6 +1403,9 @@ void __fastcall TPreferencesDialog::UpdateControls()
 
     // network
     EnableControl(CustomExternalIpAddressEdit, CustomExternalIpAddressButton->Checked);
+    EnableControl(LocalPortNumberMinEdit, LocalPortNumberCheck->Checked);
+    EnableControl(LocalPortNumberMaxEdit, LocalPortNumberCheck->Checked);
+    EnableControl(LocalPortNumberRangeLabel, LocalPortNumberCheck->Checked);
 
     // window
     EnableControl(AutoWorkspaceCombo, AutoSaveWorkspaceCheck->Checked);
@@ -3181,3 +3193,26 @@ void __fastcall TPreferencesDialog::UpDownFileColorButtonClick(TObject * Sender)
   FileColorMove(FileColorsView->ItemIndex, DestIndex);
 }
 //---------------------------------------------------------------------------
+void __fastcall TPreferencesDialog::LocalPortNumberMinEditExit(TObject *)
+{
+  if (LocalPortNumberMinEdit->AsInteger > LocalPortNumberMaxEdit->AsInteger)
+  {
+    LocalPortNumberMaxEdit->Value = LocalPortNumberMaxEdit->MaxValue;
+  }
+}
+//---------------------------------------------------------------------------
+void __fastcall TPreferencesDialog::LocalPortNumberMaxEditExit(TObject *)
+{
+  if (LocalPortNumberMaxEdit->AsInteger < LocalPortNumberMinEdit->AsInteger)
+  {
+    if (LocalPortNumberMaxEdit->AsInteger >= PrivatePortMin)
+    {
+      LocalPortNumberMinEdit->AsInteger = PrivatePortMin;
+    }
+    else
+    {
+      LocalPortNumberMinEdit->Value = LocalPortNumberMinEdit->MinValue;
+    }
+  }
+}
+//---------------------------------------------------------------------------

source/forms/Preferences.dfm

@@ -2776,17 +2776,24 @@ object PreferencesDialog: TPreferencesDialog
         DesignSize = (
           405
           398)
-        object ExternalIpAddressGroupBox: TGroupBox
+        object ExternalIpAddressGroupBox2: TGroupBox
           Left = 8
           Top = 8
           Width = 389
-          Height = 98
+          Height = 152
           Anchors = [akLeft, akTop, akRight]
-          Caption = 'External IP address'
+          Caption = 'Incoming FTP connections (active mode)'
           TabOrder = 0
           DesignSize = (
             389
-            98)
+            152)
+          object LocalPortNumberRangeLabel: TLabel
+            Left = 133
+            Top = 120
+            Width = 6
+            Height = 13
+            Caption = #8211
+          end
           object RetrieveExternalIpAddressButton: TRadioButton
             Left = 16
             Top = 21
@@ -2810,15 +2817,53 @@ object PreferencesDialog: TPreferencesDialog
           object CustomExternalIpAddressEdit: TEdit
             Left = 45
             Top = 67
-            Width = 136
+            Width = 182
             Height = 21
             TabOrder = 2
             OnClick = ControlChange
           end
+          object LocalPortNumberCheck: TCheckBox
+            Left = 16
+            Top = 94
+            Width = 361
+            Height = 17
+            Anchors = [akLeft, akTop, akRight]
+            Caption = 'Limit listening &ports to:'
+            TabOrder = 3
+            OnClick = ControlChange
+          end
+          object LocalPortNumberMinEdit: TUpDownEdit
+            Left = 45
+            Top = 117
+            Width = 82
+            Height = 21
+            Alignment = taRightJustify
+            MaxValue = 65535.000000000000000000
+            MinValue = 1024.000000000000000000
+            Value = 1.000000000000000000
+            Anchors = [akTop, akRight]
+            TabOrder = 4
+            OnChange = ControlChange
+            OnExit = LocalPortNumberMinEditExit
+          end
+          object LocalPortNumberMaxEdit: TUpDownEdit
+            Left = 145
+            Top = 117
+            Width = 82
+            Height = 21
+            Alignment = taRightJustify
+            MaxValue = 65535.000000000000000000
+            MinValue = 1024.000000000000000000
+            Value = 1.000000000000000000
+            Anchors = [akTop, akRight]
+            TabOrder = 5
+            OnChange = ControlChange
+            OnExit = LocalPortNumberMaxEditExit
+          end
         end
         object ConnectionsGroup: TGroupBox
           Left = 8
-          Top = 112
+          Top = 166
           Width = 389
           Height = 53
           Anchors = [akLeft, akTop, akRight]

source/forms/Preferences.h

@@ -205,7 +205,7 @@ __published:
   TButton *SetMasterPasswordButton;
   TCheckBox *UseMasterPasswordCheck;
   TTabSheet *NetworkSheet;
-  TGroupBox *ExternalIpAddressGroupBox;
+  TGroupBox *ExternalIpAddressGroupBox2;
   TRadioButton *RetrieveExternalIpAddressButton;
   TRadioButton *CustomExternalIpAddressButton;
   TEdit *CustomExternalIpAddressEdit;
@@ -336,6 +336,10 @@ __published:
   TComboBox *PanelSearchCombo;
   TLabel *Label2;
   TCheckBox *ShowLoginWhenNoSessionCheck;
+  TCheckBox *LocalPortNumberCheck;
+  TLabel *LocalPortNumberRangeLabel;
+  TUpDownEdit *LocalPortNumberMinEdit;
+  TUpDownEdit *LocalPortNumberMaxEdit;
   void __fastcall FormShow(TObject *Sender);
   void __fastcall ControlChange(TObject *Sender);
   void __fastcall EditorFontButtonClick(TObject *Sender);
@@ -440,6 +444,8 @@ __published:
   void __fastcall FileColorsViewDblClick(TObject *Sender);
   void __fastcall UpDownFileColorButtonClick(TObject *Sender);
   void __fastcall CopyParamListViewDragOver(TObject *Sender, TObject *Source, int X, int Y, TDragState State, bool &Accept);
+  void __fastcall LocalPortNumberMinEditExit(TObject *Sender);
+  void __fastcall LocalPortNumberMaxEditExit(TObject *Sender);
 
 private:
   TPreferencesMode FPreferencesMode;