Bug 1578: Mask out passwords in raw settings (proxy and tunnel) when logging command-line (2nd try for logging suggested "open" command) + Do not munge * in password (or in general in URL)

https://winscp.net/tracker/1578

Source commit: 659e267edb785ef4c69e07d3e76d23048709da9a

source/core/Common.cpp

@@ -2486,7 +2486,7 @@ UnicodeString __fastcall DoEncodeUrl(UnicodeString S, bool EncodeSlash)
     wchar_t C = S[Index];
     if (IsLetter(C) ||
         IsDigit(C) ||
-        (C == L'_') || (C == L'-') || (C == L'.') ||
+        (C == L'_') || (C == L'-') || (C == L'.') || (C == L'*') ||
         ((C == L'/') && !EncodeSlash))
     {
       Index++;

source/core/HierarchicalStorage.cpp

@@ -20,7 +20,7 @@
 #define WRITE_REGISTRY(Method) \
   try { FRegistry->Method(Name, Value); } catch(...) { FFailed++; }
 //---------------------------------------------------------------------------
-UnicodeString __fastcall MungeStr(const UnicodeString & Str, bool ForceAnsi)
+UnicodeString __fastcall MungeStr(const UnicodeString & Str, bool ForceAnsi, bool Value)
 {
   RawByteString Source;
   if (ForceAnsi)
@@ -40,6 +40,11 @@ UnicodeString __fastcall MungeStr(const UnicodeString & Str, bool ForceAnsi)
   Dest.SetLength(Source.Length() * 3 + 1);
   putty_mungestr(Source.c_str(), Dest.c_str());
   PackStr(Dest);
+  if (Value)
+  {
+    // We do not want to munge * in PasswordMask
+    Dest = ReplaceStr(Dest, L"%2A", L"*");
+  }
   return UnicodeString(Dest.c_str(), Dest.Length());
 }
 //---------------------------------------------------------------------------
@@ -67,7 +72,7 @@ UnicodeString __fastcall UnMungeStr(const UnicodeString & Str)
 //---------------------------------------------------------------------------
 UnicodeString __fastcall PuttyMungeStr(const UnicodeString Str)
 {
-  return MungeStr(Str, false);
+  return MungeStr(Str, false, false);
 }
 //---------------------------------------------------------------------------
 UnicodeString __fastcall MungeIniName(const UnicodeString & Str)
@@ -143,11 +148,11 @@ bool __fastcall THierarchicalStorage::OpenRootKey(bool CanCreate)
 //---------------------------------------------------------------------------
 UnicodeString __fastcall THierarchicalStorage::MungeKeyName(UnicodeString Key)
 {
-  UnicodeString Result = MungeStr(Key, ForceAnsi);
+  UnicodeString Result = MungeStr(Key, ForceAnsi, false);
   // if there's already ANSI-munged subkey, keep ANSI munging
   if ((Result != Key) && !ForceAnsi && DoKeyExists(Key, true))
   {
-    Result = MungeStr(Key, true);
+    Result = MungeStr(Key, true, false);
   }
   return Result;
 }
@@ -333,7 +338,7 @@ UnicodeString __fastcall THierarchicalStorage::ReadString(const UnicodeString Na
   UnicodeString Result;
   if (MungeStringValues)
   {
-    Result = UnMungeStr(ReadStringRaw(Name, MungeStr(Default, ForceAnsi)));
+    Result = UnMungeStr(ReadStringRaw(Name, MungeStr(Default, ForceAnsi, true)));
   }
   else
   {
@@ -367,7 +372,7 @@ void __fastcall THierarchicalStorage::WriteString(const UnicodeString Name, cons
 {
   if (MungeStringValues)
   {
-    WriteStringRaw(Name, MungeStr(Value, ForceAnsi));
+    WriteStringRaw(Name, MungeStr(Value, ForceAnsi, true));
   }
   else
   {
@@ -457,7 +462,7 @@ bool __fastcall TRegistryStorage::Copy(TRegistryStorage * Storage)
     int Index = 0;
     while ((Index < Names->Count) && Result)
     {
-      UnicodeString Name = MungeStr(Names->Strings[Index], ForceAnsi);
+      UnicodeString Name = MungeStr(Names->Strings[Index], ForceAnsi, false);
       unsigned long Size = Buffer.size();
       unsigned long Type;
       int RegResult;
@@ -558,7 +563,7 @@ bool __fastcall TRegistryStorage::DeleteValue(const UnicodeString Name)
 //---------------------------------------------------------------------------
 bool __fastcall TRegistryStorage::DoKeyExists(const UnicodeString SubKey, bool AForceAnsi)
 {
-  UnicodeString K = MungeStr(SubKey, AForceAnsi);
+  UnicodeString K = MungeStr(SubKey, AForceAnsi, false);
   bool Result = FRegistry->KeyExists(K);
   return Result;
 }
@@ -894,7 +899,7 @@ bool __fastcall TCustomIniFileStorage::DoKeyExists(const UnicodeString SubKey, b
 {
   return
     (HandleByMasterStorage() && FMasterStorage->DoKeyExists(SubKey, AForceAnsi)) ||
-    FIniFile->SectionExists(CurrentSubKey + MungeStr(SubKey, AForceAnsi));
+    FIniFile->SectionExists(CurrentSubKey + MungeStr(SubKey, AForceAnsi, false));
 }
 //---------------------------------------------------------------------------
 bool __fastcall TCustomIniFileStorage::DoValueExists(const UnicodeString & Value)

source/core/Script.cpp

@@ -2631,10 +2631,7 @@ void __fastcall TManagementScript::Connect(const UnicodeString Session,
       {
         std::unique_ptr<TSessionData> DataWithFingerprint(Data->Clone());
         DataWithFingerprint->LookupLastFingerprint();
-        if (!DataWithFingerprint->Password.IsEmpty())
-        {
-          DataWithFingerprint->Password = PasswordMask;
-        }
+        DataWithFingerprint->MaskPasswords();
 
         PrintLine(LoadStr(SCRIPT_SITE_WARNING));
         PrintLine(L"open " + DataWithFingerprint->GenerateOpenCommandArgs(false));

source/core/SessionData.cpp

@@ -1652,6 +1652,30 @@ bool __fastcall TSessionData::MaskPasswordInOptionParameter(const UnicodeString
   return Result;
 }
 //---------------------------------------------------------------------
+void __fastcall TSessionData::MaskPasswords()
+{
+  if (!Password.IsEmpty())
+  {
+    Password = PasswordMask;
+  }
+  if (!NewPassword.IsEmpty())
+  {
+    NewPassword = PasswordMask;
+  }
+  if (!ProxyPassword.IsEmpty())
+  {
+    ProxyPassword = PasswordMask;
+  }
+  if (!TunnelPassword.IsEmpty())
+  {
+    TunnelPassword = PasswordMask;
+  }
+  if (!Passphrase.IsEmpty())
+  {
+    Passphrase = PasswordMask;
+  }
+}
+//---------------------------------------------------------------------
 bool __fastcall TSessionData::ParseUrl(UnicodeString Url, TOptions * Options,
   TStoredSessionList * StoredSessions, bool & DefaultsOnly, UnicodeString * FileName,
   bool * AProtocolDefined, UnicodeString * MaskedUrl)

source/core/SessionData.h

@@ -454,6 +454,7 @@ public:
   bool __fastcall HasAnySessionPassword();
   bool __fastcall HasAnyPassword();
   void __fastcall ClearSessionPasswords();
+  void __fastcall MaskPasswords();
   void __fastcall Remove();
   void __fastcall CacheHostKeyIfNotCached();
   virtual void __fastcall Assign(TPersistent * Source);