From 1636165e08ee65b7612b6dc447fcc6ebb7af1a72 Mon Sep 17 00:00:00 2001 From: Martin Prikryl Date: Wed, 10 Jun 2026 13:45:39 +0000 Subject: [PATCH] Warning when pasting a session URL with unsafe settings Source commit: a6a8ab80fab4384666d8f4835e7f5fefb91c4aa9 --- source/core/SessionData.cpp | 53 +++++++++++++++++++++++------- source/core/SessionData.h | 13 +++++--- source/forms/CustomScpExplorer.cpp | 10 ++++-- source/forms/Login.cpp | 16 ++++++--- source/forms/Login.h | 2 +- source/resource/TextsWin.h | 1 + source/resource/TextsWin1.rc | 1 + source/windows/ConsoleRunner.cpp | 3 +- 8 files changed, 74 insertions(+), 25 deletions(-) diff --git a/source/core/SessionData.cpp b/source/core/SessionData.cpp index 809ca9709..3bc14dd1e 100644 --- a/source/core/SessionData.cpp +++ b/source/core/SessionData.cpp @@ -134,6 +134,7 @@ __fastcall TSessionData::TSessionData(UnicodeString aName): { Default(); FModified = true; + FUnsafeSettings = nullptr; } //--------------------------------------------------------------------- _fastcall TSessionData::~TSessionData() @@ -687,11 +688,15 @@ bool __fastcall TSessionData::IsInFolderOrWorkspace(const UnicodeString & AFolde return StartsText(UnixIncludeTrailingBackslash(AFolder), Name); } //--------------------------------------------------------------------- -void __fastcall TSessionData::DoLoad(THierarchicalStorage * Storage, bool PuttyImport, bool & RewritePassword, bool Unsafe, bool RespectDisablePasswordStoring) +void TSessionData::DoLoad( + THierarchicalStorage * Storage, bool PuttyImport, bool & RewritePassword, + bool Unsafe, bool RespectDisablePasswordStoring, bool & UnsafeSettings) { // Make sure we only ever use methods supported by TOptionsStorage // (implemented by TOptionsIniFile) + UnsafeSettings = false; + PortNumber = Storage->ReadInteger(L"PortNumber", PortNumber); UserName = Storage->ReadString(L"UserName", UserName); // must be loaded after UserName, because HostName may be in format user@host @@ -709,6 +714,7 @@ void __fastcall TSessionData::DoLoad(THierarchicalStorage * Storage, bool PuttyI SET_SESSION_PROPERTY_FROM(PROP, A##PROP); \ } #define LOAD_PASSWORD(PROP, PLAIN_NAME) LOAD_PASSWORD_EX(PROP, PLAIN_NAME, TEXT(#PROP), RewritePassword = true;) + #define LOADING_UNSAFE TValueRestorer UnsafeSettingsRestorer(FUnsafeSettings, &UnsafeSettings); bool LoadPasswords = !Configuration->DisablePasswordStoring || !RespectDisablePasswordStoring; if (LoadPasswords) { @@ -754,6 +760,7 @@ void __fastcall TSessionData::DoLoad(THierarchicalStorage * Storage, bool PuttyI HostKeyList = Storage->ReadString(L"HostKey", HostKeyList); if (!Unsafe) { + LOADING_UNSAFE; GssLibList = Storage->ReadString(L"GSSLibs", GssLibList); } GssLibCustom = Storage->ReadString(L"GSSCustom", GssLibCustom); @@ -780,12 +787,14 @@ void __fastcall TSessionData::DoLoad(THierarchicalStorage * Storage, bool PuttyI Special = Storage->ReadBool(L"Special", Special); if (!Unsafe) { + LOADING_UNSAFE; Shell = Storage->ReadString(L"Shell", Shell); } ClearAliases = Storage->ReadBool(L"ClearAliases", ClearAliases); UnsetNationalVars = Storage->ReadBool(L"UnsetNationalVars", UnsetNationalVars); if (!Unsafe) { + LOADING_UNSAFE; ListingCommand = Storage->ReadString(L"ListingCommand", Storage->ReadBool(L"AliasGroupList", false) ? UnicodeString(L"ls -gla") : ListingCommand); } @@ -796,6 +805,7 @@ void __fastcall TSessionData::DoLoad(THierarchicalStorage * Storage, bool PuttyI TimeDifferenceAuto = Storage->ReadBool(L"TimeDifferenceAuto", (TimeDifference == TDateTime())); if (!Unsafe) { + LOADING_UNSAFE; DeleteToRecycleBin = Storage->ReadBool(L"DeleteToRecycleBin", DeleteToRecycleBin); OverwrittenToRecycleBin = Storage->ReadBool(L"OverwrittenToRecycleBin", OverwrittenToRecycleBin); RecycleBinPath = Storage->ReadString(L"RecycleBinPath", RecycleBinPath); @@ -839,6 +849,7 @@ void __fastcall TSessionData::DoLoad(THierarchicalStorage * Storage, bool PuttyI LOAD_PASSWORD_EX(ProxyPassword, L"ProxyPassword", L"ProxyPasswordEnc", ); if (!Unsafe) { + LOADING_UNSAFE; if (ProxyMethod == pmCmd) { ProxyLocalCommand = Storage->ReadStringRaw(L"ProxyTelnetCommand", ProxyLocalCommand); @@ -874,6 +885,7 @@ void __fastcall TSessionData::DoLoad(THierarchicalStorage * Storage, bool PuttyI if (!Unsafe) { + LOADING_UNSAFE; SftpServer = Storage->ReadString(L"SftpServer", SftpServer); } #define READ_SFTP_BUG(BUG) \ @@ -962,6 +974,7 @@ void __fastcall TSessionData::DoLoad(THierarchicalStorage * Storage, bool PuttyI CustomParam1 = Storage->ReadString(L"CustomParam1", CustomParam1); CustomParam2 = Storage->ReadString(L"CustomParam2", CustomParam2); + #undef LOADING_UNSAFE #undef LOAD_PASSWORD } //--------------------------------------------------------------------- @@ -978,7 +991,8 @@ void __fastcall TSessionData::Load(THierarchicalStorage * Storage, bool PuttyImp ClearSessionPasswords(); FProxyPassword = L""; - DoLoad(Storage, PuttyImport, RewritePassword, false, true); + bool UnsafeSettings; // unused + DoLoad(Storage, PuttyImport, RewritePassword, false, true, UnsafeSettings); Storage->CloseSubKey(); } @@ -1958,6 +1972,10 @@ void __fastcall TSessionData::Modify() { FSource = ssStoredModified; } + if (FUnsafeSettings != nullptr) + { + *FUnsafeSettings = true; + } } //--------------------------------------------------------------------- UnicodeString __fastcall TSessionData::GetSourceName() @@ -2434,7 +2452,7 @@ bool TSessionData::ParseUrl( if (RawSettings->Count > 0) // optimization { - ApplyRawSettings(RawSettings.get(), Unsafe); + ApplyRawSettings(RawSettings.get(), Unsafe, ParsedInfo); } bool HasPassword = (UserInfo.Pos(L':') > 0); @@ -2591,7 +2609,7 @@ bool TSessionData::ParseUrl( std::unique_ptr RawSettings(new TStringList()); if (Options->FindSwitch(RawSettingsOption, RawSettings.get())) { - ApplyRawSettings(RawSettings.get(), Unsafe); + ApplyRawSettings(RawSettings.get(), Unsafe, ParsedInfo); } } if (Options->FindSwitch(PASSWORDSFROMFILES_SWITCH) && @@ -2607,16 +2625,28 @@ bool TSessionData::ParseUrl( return true; } //--------------------------------------------------------------------- -void __fastcall TSessionData::ApplyRawSettings(TStrings * RawSettings, bool Unsafe) +void TSessionData::ApplyRawSettings(TStrings * RawSettings, bool Unsafe) { - std::unique_ptr OptionsStorage(new TOptionsStorage(RawSettings, false)); - ApplyRawSettings(OptionsStorage.get(), Unsafe, false); + int ParsedInfo; + ApplyRawSettings(RawSettings, Unsafe, ParsedInfo); } //--------------------------------------------------------------------- -void __fastcall TSessionData::ApplyRawSettings(THierarchicalStorage * Storage, bool Unsafe, bool RespectDisablePasswordStoring) +void TSessionData::ApplyRawSettings(TStrings * RawSettings, bool Unsafe, int & ParsedInfo) +{ + std::unique_ptr OptionsStorage(new TOptionsStorage(RawSettings, false)); + bool UnsafeSettings; + ApplyRawSettings(OptionsStorage.get(), Unsafe, false, UnsafeSettings); + if (UnsafeSettings) + { + ParsedInfo |= piUnsafeSettings; + } +} +//--------------------------------------------------------------------- +void TSessionData::ApplyRawSettings( + THierarchicalStorage * Storage, bool Unsafe, bool RespectDisablePasswordStoring, bool & UnsafeSettings) { bool Dummy; - DoLoad(Storage, false, Dummy, Unsafe, RespectDisablePasswordStoring); + DoLoad(Storage, false, Dummy, Unsafe, RespectDisablePasswordStoring, UnsafeSettings); } //--------------------------------------------------------------------- void __fastcall TSessionData::ConfigureTunnel(int APortNumber) @@ -5787,12 +5817,11 @@ TSessionData * TStoredSessionList::ParseUrl( return Data.release(); } //--------------------------------------------------------------------- -bool __fastcall TStoredSessionList::IsUrl(UnicodeString Url) +int TStoredSessionList::GetUrlInfo(const UnicodeString & Url) { int ParsedInfo; std::unique_ptr ParsedData(ParseUrl(Url, NULL, ParsedInfo)); - bool Result = FLAGSET(ParsedInfo, piProtocolDefined); - return Result; + return ParsedInfo; } //--------------------------------------------------------------------- TSessionData * __fastcall TStoredSessionList::ResolveWorkspaceData(TSessionData * Data) diff --git a/source/core/SessionData.h b/source/core/SessionData.h index fd1628c7d..fbb9b46d5 100644 --- a/source/core/SessionData.h +++ b/source/core/SessionData.h @@ -64,6 +64,7 @@ enum TParsedInfoFlags { piDefaultsOnly = 0x01, piProtocolDefined = 0x02, + piUnsafeSettings = 0x04, }; //--------------------------------------------------------------------------- extern const UnicodeString CipherNames[CIPHER_COUNT]; @@ -265,6 +266,7 @@ private: TSessionSource FSource; bool FSaveOnly; UnicodeString FLogicalHostName; + bool * FUnsafeSettings; void __fastcall SetHostName(UnicodeString value); UnicodeString __fastcall GetHostNameExpanded(); @@ -462,7 +464,9 @@ private: UnicodeString __fastcall GetFolderName(); void __fastcall Modify(); UnicodeString __fastcall GetSourceName(); - void __fastcall DoLoad(THierarchicalStorage * Storage, bool PuttyImport, bool & RewritePassword, bool Unsafe, bool RespectDisablePasswordStoring); + void DoLoad( + THierarchicalStorage * Storage, bool PuttyImport, bool & RewritePassword, + bool Unsafe, bool RespectDisablePasswordStoring, bool & UnsafeSettings); void __fastcall DoSave(THierarchicalStorage * Storage, bool PuttyExport, const TSessionData * Default, bool DoNotEncryptPasswords); UnicodeString __fastcall ReadXmlNode(_di_IXMLNode Node, const UnicodeString & Name, const UnicodeString & Default); @@ -506,6 +510,7 @@ private: void __fastcall SetAlgoList(AlgoT * List, const AlgoT * DefaultList, const UnicodeString * Names, int Count, AlgoT WarnAlgo, UnicodeString value); void DefaultProxy(); + void ApplyRawSettings(TStrings * RawSettings, bool Unsafe, int & ParsedInfo); static void __fastcall Remove(THierarchicalStorage * Storage, const UnicodeString & Name); __property UnicodeString InternalStorageKey = { read = GetInternalStorageKey }; @@ -518,8 +523,8 @@ public: void __fastcall DefaultSettings(); void __fastcall NonPersistent(); void __fastcall Load(THierarchicalStorage * Storage, bool PuttyImport); - void __fastcall ApplyRawSettings(TStrings * RawSettings, bool Unsafe); - void __fastcall ApplyRawSettings(THierarchicalStorage * Storage, bool Unsafe, bool RespectDisablePasswordStoring); + void ApplyRawSettings(TStrings * RawSettings, bool Unsafe); + void ApplyRawSettings(THierarchicalStorage * Storage, bool Unsafe, bool RespectDisablePasswordStoring, bool & UnsafeSettings); void __fastcall ImportFromFilezilla(_di_IXMLNode Node, const UnicodeString & Path, _di_IXMLNode SettingsNode); void ImportFromOpenssh(TStrings * Lines); void __fastcall Save(THierarchicalStorage * Storage, bool PuttyExport, @@ -789,7 +794,7 @@ public: TSessionData * ParseUrl( const UnicodeString & Url, TOptions * Options, int & ParsedInfo, UnicodeString * FileName = NULL, UnicodeString * MaskedUrl = NULL, int Flags = 0); - bool __fastcall IsUrl(UnicodeString Url); + int GetUrlInfo(const UnicodeString & Url); bool __fastcall CanOpen(TSessionData * Data); void __fastcall GetFolderOrWorkspace(const UnicodeString & Name, TList * List); TStrings * __fastcall GetFolderOrWorkspaceList(const UnicodeString & Name); diff --git a/source/forms/CustomScpExplorer.cpp b/source/forms/CustomScpExplorer.cpp index 0b2ef8c49..41fb9e569 100644 --- a/source/forms/CustomScpExplorer.cpp +++ b/source/forms/CustomScpExplorer.cpp @@ -9273,7 +9273,7 @@ bool __fastcall TCustomScpExplorerForm::CanPasteFromClipBoard() if (NonEmptyTextFromClipboard(ClipboardText) && (ClipboardText.Pos(L"\n") == 0)) // it's already trimmed { - if (StoredSessions->IsUrl(ClipboardText)) + if (FLAGSET(StoredSessions->GetUrlInfo(ClipboardText), piProtocolDefined)) { Result = true; } @@ -9311,8 +9311,14 @@ void __fastcall TCustomScpExplorerForm::PasteFromClipBoard() UnicodeString ClipboardText; if (NonEmptyTextFromClipboard(ClipboardText)) { - if (StoredSessions->IsUrl(ClipboardText)) + int ParsedInfo = StoredSessions->GetUrlInfo(ClipboardText); + if (FLAGSET(ParsedInfo, piProtocolDefined)) { + if (FLAGSET(ParsedInfo, piUnsafeSettings) && + (MessageDialog(LoadStr(UNSAFE_SESSION), qtConfirmation, qaOK | qaCancel, HELP_NONE) != qaOK)) + { + Abort(); + } NewSession(ClipboardText); } else diff --git a/source/forms/Login.cpp b/source/forms/Login.cpp index a7609884b..d4c2c412e 100644 --- a/source/forms/Login.cpp +++ b/source/forms/Login.cpp @@ -1285,7 +1285,7 @@ void __fastcall TLoginDialog::ActionListUpdate(TBasicAction * BasicAction, UnicodeString ClipboardUrl; Action->Enabled = NonEmptyTextFromClipboard(ClipboardUrl) && - StoredSessions->IsUrl(ClipboardUrl); + FLAGSET(StoredSessions->GetUrlInfo(ClipboardUrl), piProtocolDefined); } else if (Action == GenerateUrlAction2) { @@ -3179,11 +3179,10 @@ void __fastcall TLoginDialog::LoginButtonDropDownClick(TObject * /*Sender*/) MenuPopup(LoginDropDownMenu, LoginButton); } //--------------------------------------------------------------------------- -void TLoginDialog::DoParseUrl(TSessionData * SessionData, const UnicodeString & Url) +void TLoginDialog::DoParseUrl(TSessionData * SessionData, const UnicodeString & Url, int & ParsedInfo) { // We do not want to pass in StoredSessions as we do not want the URL be // parsed as pointing to a stored site. - int ParsedInfo; // unused SessionData->ParseUrl(Url, NULL, NULL, ParsedInfo, NULL, NULL, pufPreferProtocol); SessionData->RequireDirectories = false; } @@ -3206,7 +3205,13 @@ void __fastcall TLoginDialog::ParseUrl(const UnicodeString & Url) } else { - DoParseUrl(SessionData.get(), Url); + int ParsedInfo; + DoParseUrl(SessionData.get(), Url, ParsedInfo); + if (FLAGSET(ParsedInfo, piUnsafeSettings) && + (MessageDialog(LoadStr(UNSAFE_SESSION), qtConfirmation, qaOK | qaCancel, HELP_NONE) != qaOK)) + { + Abort(); + } } LoadSession(SessionData.get()); @@ -3242,7 +3247,8 @@ void __fastcall TLoginDialog::ParseHostName() { // All this check is probably unnecessary, keeping it just to be safe std::unique_ptr SessionData(new TSessionData(EmptyStr)); - DoParseUrl(SessionData.get(), HostName); + int ParsedInfo; // unused + DoParseUrl(SessionData.get(), HostName, ParsedInfo); std::unique_ptr HostNameSessionData(new TSessionData(EmptyStr)); HostNameSessionData->HostName = HostName; if ((HostNameSessionData->HostName != HostName) || // Has legacy HostName property parsing intervened? diff --git a/source/forms/Login.h b/source/forms/Login.h index 012fbc85a..66fcce998 100644 --- a/source/forms/Login.h +++ b/source/forms/Login.h @@ -414,7 +414,7 @@ private: bool __fastcall IsEditable(); TSessionData * __fastcall CloneSelectedSession(); void __fastcall CloneToNewSite(); - void DoParseUrl(TSessionData * SessionData, const UnicodeString & Url); + void DoParseUrl(TSessionData * SessionData, const UnicodeString & Url, int & ParsedInfo); void __fastcall ParseUrl(const UnicodeString & Url); void __fastcall ParseHostName(); void __fastcall ResetNewSiteData(); diff --git a/source/resource/TextsWin.h b/source/resource/TextsWin.h index 58fb0e5b2..792e77ca9 100644 --- a/source/resource/TextsWin.h +++ b/source/resource/TextsWin.h @@ -94,6 +94,7 @@ #define COPYID_IDETITY_MISSING 1213 #define TAG_NOT_UNIQUE 1214 #define TAG_INVALID_CHAR 1215 +#define UNSAFE_SESSION 1216 #define WIN_CONFIRMATION_STRINGS 1300 #define CONFIRM_OVERWRITE_SESSION 1301 diff --git a/source/resource/TextsWin1.rc b/source/resource/TextsWin1.rc index 26510b131..f57d8b41e 100644 --- a/source/resource/TextsWin1.rc +++ b/source/resource/TextsWin1.rc @@ -102,6 +102,7 @@ BEGIN COPYID_IDETITY_MISSING, "Identity/key file was not specified." TAG_NOT_UNIQUE, "Keys must be unique." TAG_INVALID_CHAR, "Character \"%s\" is not allowed in tags." + UNSAFE_SESSION, "**Use potentially unsafe session settings?**\nThe provided session URL contains potentially unsafe settings. Proceed only if you trust its source." WIN_CONFIRMATION_STRINGS, "WIN_CONFIRMATION" CONFIRM_OVERWRITE_SESSION, "Site with name '%s' already exists. Overwrite?" diff --git a/source/windows/ConsoleRunner.cpp b/source/windows/ConsoleRunner.cpp index e9229c899..3259cbdb3 100644 --- a/source/windows/ConsoleRunner.cpp +++ b/source/windows/ConsoleRunner.cpp @@ -2498,7 +2498,8 @@ int __fastcall BatchSettings(TConsole * Console, TProgramParams * Params) Matches++; std::unique_ptr OriginalData(new TSessionData(L"")); OriginalData->CopyDataNoRecrypt(Data); - Data->ApplyRawSettings(OptionsStorage.get(), false, true); + bool UnsafeSettings; // unused + Data->ApplyRawSettings(OptionsStorage.get(), false, true, UnsafeSettings); bool Changed = !OriginalData->IsSame(Data, false); if (Changed) {