sysutils/winscp
1
0
Fork 0
mirror of https://github.com/winscp/winscp.git synced 2026-10-24 02:28:55 +03:00
Code Activity

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

Browse Source 
https://winscp.net/tracker/1943

Source commit: 5eb66f66fd80c93d50b234dcb17254c7aa78930e
This commit is contained in:
Martin Prikryl
2021-01-21 14:50:18 +00:00
parent 2dd517c888
commit ec584f5189
5 changed files with 53 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

67
source/core/SessionData.cpp
View File

@@ -617,7 +617,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)
@@ -690,7 +690,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>
@@ -713,22 +716,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 = Storage->ReadEnum(L"SCPLsFullTime", SCPLsFullTime, AutoSwitchMapping);
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 = Storage->ReadEnum(L"LookupUserGroups2", LookupUserGroups, AutoSwitchMapping);
EOLType = (TEOLType)Storage->ReadInteger(L"EOLType", EOLType);
@@ -767,13 +779,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);
@@ -802,7 +817,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] = Storage->ReadEnum(L"SFTP" #BUG "Bug", SFTPBug[sb##BUG], AutoSwitchMapping);
READ_SFTP_BUG(Symlink);
@@ -950,7 +968,7 @@ void __fastcall TSessionData::Load(THierarchicalStorage * Storage, bool PuttyImp
ClearSessionPasswords();
FProxyPassword = L"";
DoLoad(Storage, PuttyImport, RewritePassword);
DoLoad(Storage, PuttyImport, RewritePassword, false);
Storage->CloseSubKey();
}
@@ -1981,6 +1999,7 @@ bool __fastcall TSessionData::ParseUrl(UnicodeString Url, TOptions * Options,
*AProtocolDefined = ProtocolDefined;
}
bool Unsafe = FLAGSET(Flags, pufUnsafe);
if (!Url.IsEmpty())
{
UnicodeString DecodedUrl = DecodeUrlChars(Url);
@@ -2145,7 +2164,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);
@@ -2302,7 +2321,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);
}
}
}
@@ -2310,16 +2329,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)

7
source/core/SessionData.h
View File

@@ -54,6 +54,7 @@ enum TSessionUrlFlags
enum TParseUrlFlags
{
pufAllowStoredSiteWithProtocol = 0x01,
pufUnsafe = 0x02,
};
//---------------------------------------------------------------------------
extern const UnicodeString CipherNames[CIPHER_COUNT];
@@ -434,7 +435,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);
@@ -488,8 +489,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);

2
source/forms/Custom.cpp
View File

@@ -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;
}

2
source/windows/ConsoleRunner.cpp
View File

@@ -2491,7 +2491,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)
{

5
source/windows/WinMain.cpp
View File

@@ -1129,7 +1129,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)))
{