Crash when starting Project Monitor

[Daniel Sahlberg]

Hi,

I've encountered a crash when starting Project Monitor (or probably rather when it does the first check of the monitored repos).

I've debugged it and the crash occurs in LogDlg.Cpp:

                callback->SetAuthData(CStringUtils::Decrypt(item.userName).get(), CStringUtils::Decrypt(item.password).get());

From what I can tell, the call to CryptUnprotectData fails, returning a nullptr and the call to .get() above fails.

    if (CryptUnprotectData(&blobIn, &descr, nullptr, nullptr, nullptr, CRYPTPROTECT_UI_FORBIDDEN, &blobOut) == FALSE)
        return nullptr;

MonitoringData.ini contains the username and password (among other things):

[item_000]

password=01 00 [...] d7

username=01 00 [...] 19

I can also see that this read in correctly and used in the call to CStringUtils::Decrypt() so I can only assume that something has changed which caused the encrypted data to become invalid. Granted, this is invalid data, but it shouldn't cause a crash.

How about the following change?

[[[

--- C:/Users/daniel/AppData/Local/Temp/LogDlg.cpp-revBASE.svn001.tmp.cpp fre sep 15 19:59:11 2023
+++ C:/devel/tsvn_trunk/src/TortoiseProc/LogDialog/LogDlg.cpp tor mar  6 13:55:49 2025
@@ -8723,8 +8723,10 @@ void CLogDlg::MonitorEditProject(MonitorItem* pPro
     {
         dlg.m_sName           = pProject->name;
         dlg.m_sPathOrURL      = pProject->wcPathOrUrl;
-        dlg.m_sUsername       = CStringUtils::Decrypt(pProject->userName).get();
-        dlg.m_sPassword       = CStringUtils::Decrypt(pProject->password).get();
+        auto username         = CStringUtils::Decrypt(pProject->userName);
+        dlg.m_sUsername       = username == nullptr ? CString("") : username.get();
+        auto password         = CStringUtils::Decrypt(pProject->password);
+        dlg.m_sPassword       = password == nullptr ? CString("") : password.get();
         dlg.m_monitorInterval = pProject->interval;
         dlg.m_sIgnoreRegex    = pProject->sMsgRegex;
         dlg.m_isParentPath    = pProject->parentPath;
@@ -9009,7 +9011,11 @@ void CLogDlg::MonitorThread()
                 // we have to include the authentication in the URL itself
                 auto tempFile = CTempFiles::Instance().GetTempFilePath(true);
                 auto callback = std::make_unique<CCallback>();
-                callback->SetAuthData(CStringUtils::Decrypt(item.userName).get(), CStringUtils::Decrypt(item.password).get());
+                auto username = CStringUtils::Decrypt(item.userName);
+                auto password = CStringUtils::Decrypt(item.password);
+                if (username == nullptr || password == nullptr)
+                    continue;
+                callback->SetAuthData(username.get(), password.get());
                 DeleteFile(tempFile.GetWinPath());
                 HRESULT hResUdl = URLDownloadToFile(nullptr, item.wcPathOrUrl, tempFile.GetWinPath(), 0, callback.get());
                 if (m_bCancelled)
@@ -9134,7 +9140,11 @@ void CLogDlg::MonitorThread()
                 sCheckInfo.Format(IDS_MONITOR_CHECKPROJECT, static_cast<LPCWSTR>(item.name));
                 if (!m_bCancelled)
                     SetDlgItemText(IDC_LOGINFO, sCheckInfo);
-                svn.SetAuthInfo(CStringUtils::Decrypt(item.userName).get(), CStringUtils::Decrypt(item.password).get());
+                auto username = CStringUtils::Decrypt(item.userName);
+                auto password = CStringUtils::Decrypt(item.password);
+                if (username == nullptr || password == nullptr)
+                    continue;
+                svn.SetAuthInfo(username.get(), password.get());
                 svn_revnum_t head = svn.GetHEADRevision(wcPathOrUrl, false);
                 if (m_bCancelled)
                     continue;
@@ -9293,6 +9303,8 @@ void CLogDlg::MonitorThread()
                 {
                     auto         du = CStringUtils::Decrypt(item.userName);
                     auto         dp = CStringUtils::Decrypt(item.password);
+                    if (du == nullptr || dp == nullptr)
+                        continue;
                     std::wstring sU;
                     std::wstring sP;
                     auto         pU = du.get();

]]]

I don't know if it would also make sense to set item.authFailed?

Kind regards,

Daniel

[Stefan]

On Thursday, March 6, 2025 at 1:58:16 PM UTC+1 daniel.l...@gmail.com wrote:

Hi,

I've encountered a crash when starting Project Monitor (or probably rather when it does the first check of the monitored repos).

I've debugged it and the crash occurs in LogDlg.Cpp:

                callback->SetAuthData(CStringUtils::Decrypt(item.userName).get(), CStringUtils::Decrypt(item.password).get());

From what I can tell, the call to CryptUnprotectData fails, returning a nullptr and the call to .get() above fails.

    if (CryptUnprotectData(&blobIn, &descr, nullptr, nullptr, nullptr, CRYPTPROTECT_UI_FORBIDDEN, &blobOut) == FALSE)
        return nullptr;

 

returning a nullptr when the method is declared to return an std::unique_ptr returns an empty unique_ptr. So calling .get() will never crash.

but, unlike CString which one can assign a nullptr, one can not assign a nullptr to an std string. So only that will crash.

I took only part of your patch because as mentioned it's perfectly fine to assign a nullptr to a CString.

Committed the fix in r29751

Stefan

[Daniel Sahlberg]

Great, thanks for the explanation!

Daniel