Bug 1943: Prevent loading session settings that can lead to remote code execution from handled URLs

https://winscp.net/tracker/1943
(cherry picked from commit ec584f5189a856cd79509f754722a6898045c5e0)

Source commit: 0f4be408b3f01132b00682da72d925d6c4ee649b

source/Console.cbproj

@@ -43,7 +43,7 @@
 		<ProjectType>CppConsoleApplication</ProjectType>
 		<SanitizedProjectName>Console</SanitizedProjectName>
 		<VerInfo_IncludeVerInfo>true</VerInfo_IncludeVerInfo>
-		<VerInfo_Keys>CompanyName=Martin Prikryl;FileDescription=Console interface for WinSCP;FileVersion=5.0.1.0;InternalName=console;LegalCopyright=(c) 2000-2020 Martin Prikryl;LegalTrademarks=;OriginalFilename=winscp.com;ProductName=WinSCP;ProductVersion=5.17.10.0;ReleaseType=stable;WWW=https://winscp.net/</VerInfo_Keys>
+		<VerInfo_Keys>CompanyName=Martin Prikryl;FileDescription=Console interface for WinSCP;FileVersion=5.0.1.0;InternalName=console;LegalCopyright=(c) 2000-2021 Martin Prikryl;LegalTrademarks=;OriginalFilename=winscp.com;ProductName=WinSCP;ProductVersion=5.17.10.0;ReleaseType=stable;WWW=https://winscp.net/</VerInfo_Keys>
 		<VerInfo_Locale>1033</VerInfo_Locale>
 		<VerInfo_MajorVer>5</VerInfo_MajorVer>
 		<VerInfo_MinorVer>0</VerInfo_MinorVer>

source/DragExt.cbproj

@@ -49,7 +49,7 @@
 		<SanitizedProjectName>DragExt</SanitizedProjectName>
 		<VerInfo_DLL>true</VerInfo_DLL>
 		<VerInfo_IncludeVerInfo>true</VerInfo_IncludeVerInfo>
-		<VerInfo_Keys>CompanyName=Martin Prikryl;FileDescription=Drag&amp;Drop shell extension for WinSCP ($(Platform));FileVersion=2.0.0.0;InternalName=dragext;LegalCopyright=(c) 2000-2020 Martin Prikryl;LegalTrademarks=;OriginalFilename=dragext.dll;ProductName=WinSCP;ProductVersion=5.17.10.0;ReleaseType=stable;WWW=https://winscp.net/</VerInfo_Keys>
+		<VerInfo_Keys>CompanyName=Martin Prikryl;FileDescription=Drag&amp;Drop shell extension for WinSCP ($(Platform));FileVersion=2.0.0.0;InternalName=dragext;LegalCopyright=(c) 2000-2021 Martin Prikryl;LegalTrademarks=;OriginalFilename=dragext.dll;ProductName=WinSCP;ProductVersion=5.17.10.0;ReleaseType=stable;WWW=https://winscp.net/</VerInfo_Keys>
 		<VerInfo_Locale>1033</VerInfo_Locale>
 		<VerInfo_MajorVer>2</VerInfo_MajorVer>
 	</PropertyGroup>

source/WinSCP.cbproj

@@ -76,7 +76,7 @@
 		<SanitizedProjectName>WinSCP</SanitizedProjectName>
 		<UsingDelphiRTL>true</UsingDelphiRTL>
 		<VerInfo_IncludeVerInfo>true</VerInfo_IncludeVerInfo>
-		<VerInfo_Keys>CompanyName=Martin Prikryl;FileDescription=WinSCP: SFTP, FTP, WebDAV, S3 and SCP client;FileVersion=5.17.10.0;InternalName=winscp;LegalCopyright=(c) 2000-2020 Martin Prikryl;LegalTrademarks=;OriginalFilename=winscp.exe;ProductName=WinSCP;ProductVersion=5.17.10.0;ReleaseType=stable;WWW=https://winscp.net/</VerInfo_Keys>
+		<VerInfo_Keys>CompanyName=Martin Prikryl;FileDescription=WinSCP: SFTP, FTP, WebDAV, S3 and SCP client;FileVersion=5.17.10.0;InternalName=winscp;LegalCopyright=(c) 2000-2021 Martin Prikryl;LegalTrademarks=;OriginalFilename=winscp.exe;ProductName=WinSCP;ProductVersion=5.17.10.0;ReleaseType=stable;WWW=https://winscp.net/</VerInfo_Keys>
 		<VerInfo_Locale>1033</VerInfo_Locale>
 		<VerInfo_MajorVer>5</VerInfo_MajorVer>
 		<VerInfo_MinorVer>17</VerInfo_MinorVer>

source/core/SessionData.cpp

@@ -596,7 +596,7 @@ bool __fastcall TSessionData::IsInFolderOrWorkspace(UnicodeString AFolder)
   return StartsText(UnixIncludeTrailingBackslash(AFolder), Name);
 }
 //---------------------------------------------------------------------
-void __fastcall TSessionData::DoLoad(THierarchicalStorage * Storage, bool PuttyImport, bool & RewritePassword)
+void __fastcall TSessionData::DoLoad(THierarchicalStorage * Storage, bool PuttyImport, bool & RewritePassword, bool Unsafe)
 {
   // Make sure we only ever use methods supported by TOptionsStorage
   // (implemented by TOptionsIniFile)
@@ -668,7 +668,10 @@ void __fastcall TSessionData::DoLoad(THierarchicalStorage * Storage, bool PuttyI
   CipherList = Storage->ReadString(L"Cipher", CipherList);
   KexList = Storage->ReadString(L"KEX", KexList);
   HostKeyList = Storage->ReadString(L"HostKey", HostKeyList);
-  GssLibList = Storage->ReadString(L"GSSLibs", GssLibList);
+  if (!Unsafe)
+  {
+    GssLibList = Storage->ReadString(L"GSSLibs", GssLibList);
+  }
   GssLibCustom = Storage->ReadString(L"GSSCustom", GssLibCustom);
   PublicKeyFile = Storage->ReadString(L"PublicKeyFile", PublicKeyFile);
   AddressFamily = static_cast<TAddressFamily>
@@ -690,22 +693,31 @@ void __fastcall TSessionData::DoLoad(THierarchicalStorage * Storage, bool PuttyI
   DSTMode = (TDSTMode)Storage->ReadInteger(L"ConsiderDST", DSTMode);
   LockInHome = Storage->ReadBool(L"LockInHome", LockInHome);
   Special = Storage->ReadBool(L"Special", Special);
-  Shell = Storage->ReadString(L"Shell", Shell);
+  if (!Unsafe)
+  {
+    Shell = Storage->ReadString(L"Shell", Shell);
+  }
   ClearAliases = Storage->ReadBool(L"ClearAliases", ClearAliases);
   UnsetNationalVars = Storage->ReadBool(L"UnsetNationalVars", UnsetNationalVars);
-  ListingCommand = Storage->ReadString(L"ListingCommand",
-    Storage->ReadBool(L"AliasGroupList", false) ? UnicodeString(L"ls -gla") : ListingCommand);
+  if (!Unsafe)
+  {
+    ListingCommand = Storage->ReadString(L"ListingCommand",
+      Storage->ReadBool(L"AliasGroupList", false) ? UnicodeString(L"ls -gla") : ListingCommand);
+  }
   IgnoreLsWarnings = Storage->ReadBool(L"IgnoreLsWarnings", IgnoreLsWarnings);
   SCPLsFullTime = TAutoSwitch(Storage->ReadInteger(L"SCPLsFullTime", SCPLsFullTime));
   Scp1Compatibility = Storage->ReadBool(L"Scp1Compatibility", Scp1Compatibility);
   TimeDifference = Storage->ReadFloat(L"TimeDifference", TimeDifference);
   TimeDifferenceAuto = Storage->ReadBool(L"TimeDifferenceAuto", (TimeDifference == TDateTime()));
-  DeleteToRecycleBin = Storage->ReadBool(L"DeleteToRecycleBin", DeleteToRecycleBin);
-  OverwrittenToRecycleBin = Storage->ReadBool(L"OverwrittenToRecycleBin", OverwrittenToRecycleBin);
-  RecycleBinPath = Storage->ReadString(L"RecycleBinPath", RecycleBinPath);
-  PostLoginCommands = Storage->ReadString(L"PostLoginCommands", PostLoginCommands);
+  if (!Unsafe)
+  {
+    DeleteToRecycleBin = Storage->ReadBool(L"DeleteToRecycleBin", DeleteToRecycleBin);
+    OverwrittenToRecycleBin = Storage->ReadBool(L"OverwrittenToRecycleBin", OverwrittenToRecycleBin);
+    RecycleBinPath = Storage->ReadString(L"RecycleBinPath", RecycleBinPath);
+    PostLoginCommands = Storage->ReadString(L"PostLoginCommands", PostLoginCommands);
+    ReturnVar = Storage->ReadString(L"ReturnVar", ReturnVar);
+  }
 
-  ReturnVar = Storage->ReadString(L"ReturnVar", ReturnVar);
   ExitCode1IsError = Storage->ReadBool(L"ExitCode1IsError", ExitCode1IsError);
   LookupUserGroups = TAutoSwitch(Storage->ReadInteger(L"LookupUserGroups2", LookupUserGroups));
   EOLType = (TEOLType)Storage->ReadInteger(L"EOLType", EOLType);
@@ -740,13 +752,16 @@ void __fastcall TSessionData::DoLoad(THierarchicalStorage * Storage, bool PuttyI
     RawByteString AProxyPassword = Storage->ReadStringAsBinaryData(L"ProxyPasswordEnc", FProxyPassword);
     SET_SESSION_PROPERTY_FROM(ProxyPassword, AProxyPassword);
   }
-  if (ProxyMethod == pmCmd)
+  if (!Unsafe)
   {
-    ProxyLocalCommand = Storage->ReadStringRaw(L"ProxyTelnetCommand", ProxyLocalCommand);
-  }
-  else
-  {
-    ProxyTelnetCommand = Storage->ReadStringRaw(L"ProxyTelnetCommand", ProxyTelnetCommand);
+    if (ProxyMethod == pmCmd)
+    {
+      ProxyLocalCommand = Storage->ReadStringRaw(L"ProxyTelnetCommand", ProxyLocalCommand);
+    }
+    else
+    {
+      ProxyTelnetCommand = Storage->ReadStringRaw(L"ProxyTelnetCommand", ProxyTelnetCommand);
+    }
   }
   ProxyDNS = TAutoSwitch((Storage->ReadInteger(L"ProxyDNS", (ProxyDNS + 2) % 3) + 1) % 3);
   ProxyLocalhost = Storage->ReadBool(L"ProxyLocalhost", ProxyLocalhost);
@@ -775,7 +790,10 @@ void __fastcall TSessionData::DoLoad(THierarchicalStorage * Storage, bool PuttyI
       Bug[sbHMAC2] = asOn;
   }
 
-  SftpServer = Storage->ReadString(L"SftpServer", SftpServer);
+  if (!Unsafe)
+  {
+    SftpServer = Storage->ReadString(L"SftpServer", SftpServer);
+  }
   #define READ_SFTP_BUG(BUG) \
     SFTPBug[sb##BUG] = TAutoSwitch(Storage->ReadInteger(L"SFTP" #BUG "Bug", SFTPBug[sb##BUG]));
   READ_SFTP_BUG(Symlink);
@@ -920,7 +938,7 @@ void __fastcall TSessionData::Load(THierarchicalStorage * Storage, bool PuttyImp
     ClearSessionPasswords();
     FProxyPassword = L"";
 
-    DoLoad(Storage, PuttyImport, RewritePassword);
+    DoLoad(Storage, PuttyImport, RewritePassword, false);
 
     Storage->CloseSubKey();
   }
@@ -1941,6 +1959,7 @@ bool __fastcall TSessionData::ParseUrl(UnicodeString Url, TOptions * Options,
     *AProtocolDefined = ProtocolDefined;
   }
 
+  bool Unsafe = FLAGSET(Flags, pufUnsafe);
   if (!Url.IsEmpty())
   {
     UnicodeString DecodedUrl = DecodeUrlChars(Url);
@@ -2105,7 +2124,7 @@ bool __fastcall TSessionData::ParseUrl(UnicodeString Url, TOptions * Options,
 
       if (RawSettings->Count > 0) // optimization
       {
-        ApplyRawSettings(RawSettings.get());
+        ApplyRawSettings(RawSettings.get(), FLAGSET(Flags, pufUnsafe));
       }
 
       bool HasPassword = (UserInfo.Pos(L':') > 0);
@@ -2254,7 +2273,7 @@ bool __fastcall TSessionData::ParseUrl(UnicodeString Url, TOptions * Options,
       std::unique_ptr<TStrings> RawSettings(new TStringList());
       if (Options->FindSwitch(RawSettingsOption, RawSettings.get()))
       {
-        ApplyRawSettings(RawSettings.get());
+        ApplyRawSettings(RawSettings.get(), Unsafe);
       }
     }
   }
@@ -2262,16 +2281,16 @@ bool __fastcall TSessionData::ParseUrl(UnicodeString Url, TOptions * Options,
   return true;
 }
 //---------------------------------------------------------------------
-void __fastcall TSessionData::ApplyRawSettings(TStrings * RawSettings)
+void __fastcall TSessionData::ApplyRawSettings(TStrings * RawSettings, bool Unsafe)
 {
   std::unique_ptr<TOptionsStorage> OptionsStorage(new TOptionsStorage(RawSettings, false));
-  ApplyRawSettings(OptionsStorage.get());
+  ApplyRawSettings(OptionsStorage.get(), Unsafe);
 }
 //---------------------------------------------------------------------
-void __fastcall TSessionData::ApplyRawSettings(THierarchicalStorage * Storage)
+void __fastcall TSessionData::ApplyRawSettings(THierarchicalStorage * Storage, bool Unsafe)
 {
   bool Dummy;
-  DoLoad(Storage, false, Dummy);
+  DoLoad(Storage, false, Dummy, Unsafe);
 }
 //---------------------------------------------------------------------
 void __fastcall TSessionData::ConfigureTunnel(int APortNumber)

source/core/SessionData.h

@@ -55,6 +55,7 @@ enum TSessionUrlFlags
 enum TParseUrlFlags
 {
   pufAllowStoredSiteWithProtocol = 0x01,
+  pufUnsafe = 0x02,
 };
 //---------------------------------------------------------------------------
 extern const UnicodeString CipherNames[CIPHER_COUNT];
@@ -415,7 +416,7 @@ private:
   UnicodeString __fastcall GetFolderName();
   void __fastcall Modify();
   UnicodeString __fastcall GetSource();
-  void __fastcall DoLoad(THierarchicalStorage * Storage, bool PuttyImport, bool & RewritePassword);
+  void __fastcall DoLoad(THierarchicalStorage * Storage, bool PuttyImport, bool & RewritePassword, bool Unsafe);
   void __fastcall DoSave(THierarchicalStorage * Storage,
     bool PuttyExport, const TSessionData * Default, bool DoNotEncryptPasswords);
   UnicodeString __fastcall ReadXmlNode(_di_IXMLNode Node, const UnicodeString & Name, const UnicodeString & Default);
@@ -468,8 +469,8 @@ public:
   void __fastcall DefaultSettings();
   void __fastcall NonPersistant();
   void __fastcall Load(THierarchicalStorage * Storage, bool PuttyImport);
-  void __fastcall ApplyRawSettings(TStrings * RawSettings);
-  void __fastcall ApplyRawSettings(THierarchicalStorage * Storage);
+  void __fastcall ApplyRawSettings(TStrings * RawSettings, bool Unsafe);
+  void __fastcall ApplyRawSettings(THierarchicalStorage * Storage, bool Unsafe);
   void __fastcall ImportFromFilezilla(_di_IXMLNode Node, const UnicodeString & Path, _di_IXMLNode SettingsNode);
   void __fastcall Save(THierarchicalStorage * Storage, bool PuttyExport,
     const TSessionData * Default = NULL);

source/forms/Custom.cpp

@@ -1507,7 +1507,7 @@ bool __fastcall TSiteRawDialog::Execute(TSessionData * Data)
     Data->Password = BackupData->Password;
     Data->Ftps = BackupData->Ftps;
 
-    Data->ApplyRawSettings(SettingsMemo->Lines);
+    Data->ApplyRawSettings(SettingsMemo->Lines, false);
   }
   return Result;
 }

source/windows/ConsoleRunner.cpp

@@ -2368,7 +2368,7 @@ int __fastcall BatchSettings(TConsole * Console, TProgramParams * Params)
             Matches++;
             std::unique_ptr<TSessionData> OriginalData(new TSessionData(L""));
             OriginalData->CopyDataNoRecrypt(Data);
-            Data->ApplyRawSettings(OptionsStorage.get());
+            Data->ApplyRawSettings(OptionsStorage.get(), false);
             bool Changed = !OriginalData->IsSame(Data, false);
             if (Changed)
             {

source/windows/WinMain.cpp

@@ -1096,7 +1096,10 @@ int __fastcall Execute()
         std::unique_ptr<TObjectList> DataList(new TObjectList());
         try
         {
-          GetLoginData(AutoStartSession, Params, DataList.get(), DownloadFile, NeedSession, NULL, pufAllowStoredSiteWithProtocol);
+          int Flags =
+            pufAllowStoredSiteWithProtocol |
+            FLAGMASK(!CheckSafe(Params), pufUnsafe);
+          GetLoginData(AutoStartSession, Params, DataList.get(), DownloadFile, NeedSession, NULL, Flags);
           // GetLoginData now Aborts when session is needed and none is selected
           if (DebugAlwaysTrue(!NeedSession || (DataList->Count > 0)))
           {