#5947: Fix secure blob extraction code.

The blob is not null-terminated, instead there's a size parameter that needs to be respected.
codereader · Apr 24, 2022 · 277af6d · 277af6d
1 parent 0a7f391
commit 277af6d
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 3 deletions.
diff --git a/plugins/vcs/CredentialManager.h b/plugins/vcs/CredentialManager.h
@@ -25,19 +25,28 @@ class CredentialManager
     {
 #ifdef _MSC_VER
         PCREDENTIALW pcred;
-        BOOL ok = ::CredReadW(accountName.c_str(), CRED_TYPE_GENERIC, 0, &pcred);
+        auto ok = ::CredReadW(accountName.c_str(), CRED_TYPE_GENERIC, 0, &pcred);
 
         if (!ok)
         {
             return std::make_pair("", "");
         }
 
         auto user = string::unicode_to_utf8(pcred->UserName);
-        auto pw = string::unicode_to_utf8((const wchar_t*)pcred->CredentialBlob);
+
+        // Extract N characters from the credential blob
+        std::wstring blob;
+        blob.assign(reinterpret_cast<wchar_t*>(pcred->CredentialBlob), pcred->CredentialBlobSize / sizeof(wchar_t));
+
+        auto pw = string::unicode_to_utf8(blob);
+
+        // Clear out the temporary string
+        memset(blob.data(), '\0', blob.size() * sizeof(wchar_t));
 
         ::CredFree(pcred);
 
-        return std::make_pair(user, pw);
+        // Move-return the result
+        return std::make_pair(std::move(user), std::move(pw));
 #else
         return { "", "" };
 #endif

diff --git a/plugins/vcs/Remote.h b/plugins/vcs/Remote.h
@@ -124,6 +124,9 @@ class Remote final
             auto error = git_credential_userpass_plaintext_new(&credentials, userAndPass.first.c_str(), userAndPass.second.c_str());
             GitException::ThrowOnError(error);
 
+            // Clear out the password
+            memset(userAndPass.second.data(), '\0', userAndPass.second.length());
+
             return credentials;
         }