From a31e88cd64c258da80d0bab3747c407862a792b0 Mon Sep 17 00:00:00 2001 From: George Hazan Date: Tue, 13 Apr 2021 18:16:57 +0300 Subject: fixes #2836 (Miranda keeps profile password in memory after checking it) --- include/m_crypto.h | 2 +- src/core/stdcrypt/src/encrypt.cpp | 67 +++++++++++++++++++------------- src/core/stdcrypt/src/stdcrypt.h | 37 +++++++++++------- src/mir_app/src/MDatabaseCommonCrypt.cpp | 46 ++++++++++------------ 4 files changed, 84 insertions(+), 68 deletions(-) diff --git a/include/m_crypto.h b/include/m_crypto.h index 3d6e81d3a3..d13a6b053e 100644 --- a/include/m_crypto.h +++ b/include/m_crypto.h @@ -37,7 +37,7 @@ struct MICryptoEngine // get/set the instance key STDMETHOD_(size_t, getKeyLength)(void) PURE; STDMETHOD_(bool, getKey)(BYTE *pKey, size_t cbKeyLen) PURE; - STDMETHOD_(bool, setKey)(const BYTE *pKey, size_t cbKeyLen) PURE; + STDMETHOD_(bool, setKey)(const char *pszPassword, const BYTE *pKey, size_t cbKeyLen) PURE; STDMETHOD_(bool, generateKey)(void)PURE; // creates a new key inside STDMETHOD_(void, purgeKey)(void)PURE; // purges a key from memory diff --git a/src/core/stdcrypt/src/encrypt.cpp b/src/core/stdcrypt/src/encrypt.cpp index e31d29d778..27a6500f5e 100644 --- a/src/core/stdcrypt/src/encrypt.cpp +++ b/src/core/stdcrypt/src/encrypt.cpp @@ -25,16 +25,9 @@ Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. #include "../../../libs/zlib/src/zlib.h" -struct ExternalKey +CStdCrypt::CStdCrypt() { - BYTE m_key[KEY_LENGTH]; - DWORD m_crc32; - BYTE slack[BLOCK_SIZE - sizeof(DWORD)]; -}; - -CStdCrypt::CStdCrypt() : - m_password("Miranda") -{} +} void CStdCrypt::destroy() { @@ -46,42 +39,59 @@ size_t CStdCrypt::getKeyLength() return sizeof(ExternalKey); } -bool CStdCrypt::getKey(BYTE *pKey, size_t cbKeyLen) +void CStdCrypt::key2ext(const char *pszPassword, ExternalKey &key) { - if (!m_valid || cbKeyLen < sizeof(ExternalKey)) - return false; + if (mir_strlen(pszPassword) == 0) + pszPassword = "Miranda"; - ExternalKey tmp = { 0 }; + ExternalKey tmp = {}; memcpy(&tmp.m_key, m_key, KEY_LENGTH); - tmp.m_crc32 = crc32(0xAbbaDead, (LPCBYTE)m_password.GetString(), m_password.GetLength()); + tmp.m_crc32 = crc32(0xAbbaDead, (LPCBYTE)pszPassword, (int)strlen(pszPassword)); getRandomBytes(tmp.slack, sizeof(tmp.slack)); BYTE tmpHash[32]; - slow_hash(m_password, m_password.GetLength(), tmpHash); + slow_hash(pszPassword, strlen(pszPassword), tmpHash); CRijndael tmpAes; tmpAes.MakeKey(tmpHash, tmpAes.sm_chain0, KEY_LENGTH, BLOCK_SIZE); - tmpAes.Encrypt(&tmp, pKey, sizeof(tmp)); - return true; + tmpAes.Encrypt(&tmp, &key, sizeof(ExternalKey)); } -bool CStdCrypt::setKey(const BYTE *pKey, size_t cbKeyLen) +bool CStdCrypt::getKey(BYTE *pKey, size_t cbKeyLen) { - // full external key. decode & check password - if (cbKeyLen != sizeof(ExternalKey)) + if (!m_valid || cbKeyLen < sizeof(m_extKey)) return false; - + + memcpy(pKey, &m_extKey, sizeof(m_extKey)); + return true; +} + +bool CStdCrypt::checkKey(const char *pszPassword, const ExternalKey *pPublic, ExternalKey &tmp) +{ + if (mir_strlen(pszPassword) == 0) + pszPassword = "Miranda"; + BYTE tmpHash[32]; - slow_hash(m_password, m_password.GetLength(), tmpHash); + slow_hash(pszPassword, strlen(pszPassword), tmpHash); CRijndael tmpAes; tmpAes.MakeKey(tmpHash, tmpAes.sm_chain0, KEY_LENGTH, BLOCK_SIZE); - ExternalKey tmp = { 0 }; - tmpAes.Decrypt(pKey, &tmp, sizeof(tmp)); - if (tmp.m_crc32 != crc32(0xAbbaDead, (LPCBYTE)m_password.GetString(), m_password.GetLength())) + tmpAes.Decrypt(pPublic, &tmp, sizeof(tmp)); + return (tmp.m_crc32 == crc32(0xAbbaDead, (LPCBYTE)pszPassword, (int)strlen(pszPassword))); +} + +bool CStdCrypt::setKey(const char *pszPassword, const BYTE *pPublic, size_t cbKeyLen) +{ + // full external key. decode & check password + if (cbKeyLen != sizeof(m_extKey)) + return false; + + ExternalKey tmp = {}; + if (!checkKey(pszPassword, (const ExternalKey*)pPublic, tmp)) return false; + memcpy(&m_extKey, pPublic, sizeof(m_extKey)); memcpy(m_key, &tmp.m_key, KEY_LENGTH); m_aes.MakeKey(m_key, "Miranda", KEY_LENGTH, BLOCK_SIZE); return m_valid = true; @@ -95,6 +105,7 @@ bool CStdCrypt::generateKey(void) memcpy(m_key, tmp, KEY_LENGTH); m_aes.MakeKey(m_key, "Miranda", KEY_LENGTH, BLOCK_SIZE); + key2ext(nullptr, m_extKey); return m_valid = true; } @@ -107,13 +118,13 @@ void CStdCrypt::purgeKey(void) // checks the master password (in utf-8) bool CStdCrypt::checkPassword(const char *pszPassword) { - return m_password == pszPassword; + ExternalKey tmp; + return checkKey(pszPassword, &m_extKey, tmp); } -// sets the master password (in utf-8) void CStdCrypt::setPassword(const char *pszPassword) { - m_password = (pszPassword == NULL) ? "Miranda" : pszPassword; + key2ext(pszPassword, m_extKey); } // result must be freed using mir_free or assigned to mir_ptr diff --git a/src/core/stdcrypt/src/stdcrypt.h b/src/core/stdcrypt/src/stdcrypt.h index eeb35620b8..ce003bd27b 100644 --- a/src/core/stdcrypt/src/stdcrypt.h +++ b/src/core/stdcrypt/src/stdcrypt.h @@ -26,35 +26,44 @@ with this program; if not, write to the Free Software Foundation, Inc., #define KEY_LENGTH 32 #define BLOCK_SIZE 16 +struct ExternalKey +{ + BYTE m_key[KEY_LENGTH]; + DWORD m_crc32; + BYTE slack[BLOCK_SIZE - sizeof(DWORD)]; +}; + struct CStdCrypt : public MICryptoEngine, public MZeroedObject { CStdCrypt(); - BOOL m_valid; - CMStringA m_password; - - BYTE m_key[KEY_LENGTH]; + bool m_valid = false; + uint8_t m_key[KEY_LENGTH]; CRijndael m_aes; + ExternalKey m_extKey; + + bool checkKey(const char *pszPassword, const ExternalKey *pPublic, ExternalKey &key); + void key2ext(const char *pszPassword, ExternalKey &key); STDMETHODIMP_(void) destroy(); // get/set the instance key - STDMETHODIMP_(size_t) getKeyLength(void); - STDMETHODIMP_(bool) getKey(BYTE *pKey, size_t cbKeyLen); - STDMETHODIMP_(bool) setKey(const BYTE *pKey, size_t cbKeyLen); + STDMETHODIMP_(size_t) getKeyLength(void) override; + STDMETHODIMP_(bool) getKey(BYTE *pKey, size_t cbKeyLen) override; + STDMETHODIMP_(bool) setKey(const char *pszPassword, const BYTE *pKey, size_t cbKeyLen) override; - STDMETHODIMP_(bool) generateKey(void); // creates a new key inside - STDMETHODIMP_(void) purgeKey(void); // purges a key from memory + STDMETHODIMP_(bool) generateKey(void) override; // creates a new key inside + STDMETHODIMP_(void) purgeKey(void) override; // purges a key from memory // sets the master password (in utf-8) - STDMETHODIMP_(bool) checkPassword(const char *pszPassword); STDMETHODIMP_(void) setPassword(const char *pszPassword); + STDMETHODIMP_(bool) checkPassword(const char *pszPassword) override; // result must be freed using mir_free or assigned to mir_ptr - STDMETHODIMP_(BYTE*) encodeString(const char *src, size_t *cbResultLen); - STDMETHODIMP_(BYTE*) encodeBuffer(const void *src, size_t cbLen, size_t *cbResultLen); + STDMETHODIMP_(BYTE*) encodeString(const char *src, size_t *cbResultLen) override; + STDMETHODIMP_(BYTE*) encodeBuffer(const void *src, size_t cbLen, size_t *cbResultLen) override; // result must be freed using mir_free or assigned to ptrA/ptrW - STDMETHODIMP_(char*) decodeString(const BYTE *pBuf, size_t bufLen, size_t *cbResultLen); - STDMETHODIMP_(void*) decodeBuffer(const BYTE *pBuf, size_t bufLen, size_t *cbResultLen); + STDMETHODIMP_(char*) decodeString(const BYTE *pBuf, size_t bufLen, size_t *cbResultLen) override; + STDMETHODIMP_(void*) decodeBuffer(const BYTE *pBuf, size_t bufLen, size_t *cbResultLen) override; }; diff --git a/src/mir_app/src/MDatabaseCommonCrypt.cpp b/src/mir_app/src/MDatabaseCommonCrypt.cpp index ba9964b863..1e0258a318 100644 --- a/src/mir_app/src/MDatabaseCommonCrypt.cpp +++ b/src/mir_app/src/MDatabaseCommonCrypt.cpp @@ -118,13 +118,13 @@ __forceinline wchar_t *GetMenuTitle(bool bUsesPassword) void MDatabaseCommon::SetPassword(const wchar_t *ptszPassword) { - if (ptszPassword == nullptr || *ptszPassword == 0) { + if (mir_wstrlen(ptszPassword) == 0) { m_bUsesPassword = false; m_crypto->setPassword(nullptr); } else { m_bUsesPassword = true; - m_crypto->setPassword(T2Utf(ptszPassword)); + m_crypto->setPassword(pass_ptrA(mir_utf8encodeW(ptszPassword))); } Menu_ModifyItem(hSetPwdMenu, GetMenuTitle(m_bUsesPassword), Skin_GetIconHandle(SKINICON_OTHER_KEYS)); @@ -160,24 +160,14 @@ static bool CheckOldPassword(HWND hwndDlg, MDatabaseCommon *db) return true; } -struct DlgChangePassParam -{ - MDatabaseCommon *db; - wchar_t newPass[100]; - unsigned short wrongPass; -}; - static INT_PTR CALLBACK sttChangePassword(HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARAM lParam) { - DlgChangePassParam *param = (DlgChangePassParam*)GetWindowLongPtr(hwndDlg, GWLP_USERDATA); - wchar_t buf[100]; + MDatabaseCommon *db = (MDatabaseCommon *)GetWindowLongPtr(hwndDlg, GWLP_USERDATA); switch (uMsg) { case WM_INITDIALOG: TranslateDialogDefault(hwndDlg); SendDlgItemMessage(hwndDlg, IDC_HEADERBAR, WM_SETICON, ICON_SMALL, (LPARAM)g_plugin.getIcon(IDI_DATABASE, true)); - - param = (DlgChangePassParam*)lParam; SetWindowLongPtr(hwndDlg, GWLP_USERDATA, lParam); oldLangID = 0; @@ -200,15 +190,15 @@ static INT_PTR CALLBACK sttChangePassword(HWND hwndDlg, UINT uMsg, WPARAM wParam break; case IDREMOVE: - if (!CheckOldPassword(hwndDlg, param->db)) { + if (!CheckOldPassword(hwndDlg, db)) { LBL_Error: SendDlgItemMessage(hwndDlg, IDC_HEADERBAR, WM_NCPAINT, 0, 0); SetDlgItemTextA(hwndDlg, IDC_USERPASS1, ""); SetDlgItemTextA(hwndDlg, IDC_USERPASS2, ""); } else { - param->db->SetPassword(nullptr); - param->db->StoreCryptoKey(); + db->SetPassword(nullptr); + db->StoreCryptoKey(); EndDialog(hwndDlg, IDREMOVE); } break; @@ -221,17 +211,19 @@ LBL_Error: goto LBL_Error; } + wchar_t buf[100]; GetDlgItemText(hwndDlg, IDC_USERPASS2, buf, _countof(buf)); if (wcscmp(buf2, buf)) { SetDlgItemText(hwndDlg, IDC_HEADERBAR, TranslateT("Passwords do not match!")); goto LBL_Error; } - if (!CheckOldPassword(hwndDlg, param->db)) + if (!CheckOldPassword(hwndDlg, db)) goto LBL_Error; - param->db->SetPassword(buf2); - param->db->StoreCryptoKey(); + db->SetPassword(buf2); + db->StoreCryptoKey(); + SecureZeroMemory(buf, sizeof(buf)); SecureZeroMemory(buf2, sizeof(buf2)); EndDialog(hwndDlg, IDOK); } @@ -252,8 +244,7 @@ LBL_Error: static INT_PTR ChangePassword(void* obj, WPARAM, LPARAM) { MDatabaseCommon *db = (MDatabaseCommon*)obj; - DlgChangePassParam param = { db }; - DialogBoxParam(g_plugin.getInst(), MAKEINTRESOURCE(db->usesPassword() ? IDD_CHANGEPASS : IDD_NEWPASS), nullptr, sttChangePassword, (LPARAM)¶m); + DialogBoxParam(g_plugin.getInst(), MAKEINTRESOURCE(db->usesPassword() ? IDD_CHANGEPASS : IDD_NEWPASS), nullptr, sttChangePassword, (LPARAM)db); return 0; } @@ -379,6 +370,11 @@ public: m_timer.OnEvent = Callback(this, &CEnterPasswordDialog::OnTimer); } + ~CEnterPasswordDialog() + { + SecureZeroMemory(m_newPass, sizeof(m_newPass)); + } + bool OnInitDialog() override { m_header.SendMsg(WM_SETICON, ICON_SMALL, (LPARAM)g_plugin.getIcon(IDI_DATABASE, true)); @@ -426,16 +422,16 @@ int MDatabaseCommon::InitCrypt() MBinBuffer key; BOOL bSuccess = ReadCryptoKey(key); if (bSuccess && (key.length() == m_crypto->getKeyLength())) { - if (!m_crypto->setKey((const BYTE*)key.data(), key.length())) { + // first try to set a key without password + if (!m_crypto->setKey(nullptr, (const BYTE*)key.data(), key.length())) { CEnterPasswordDialog dlg(this); while (true) { if (!dlg.DoModal()) return 4; - m_crypto->setPassword(pass_ptrA(mir_utf8encodeW(dlg.m_newPass))); - if (m_crypto->setKey((const BYTE*)key.data(), key.length())) { + pass_ptrA szPassword(mir_utf8encodeW(dlg.m_newPass)); + if (m_crypto->setKey(szPassword, (const BYTE*)key.data(), key.length())) { m_bUsesPassword = true; - SecureZeroMemory(&dlg.m_newPass, sizeof(dlg.m_newPass)); break; } dlg.m_wrongPass++; -- cgit v1.2.3