diff --git a/src/Common/Dlgcode.c b/src/Common/Dlgcode.c index 15137e38..0581d7ed 100644 --- a/src/Common/Dlgcode.c +++ b/src/Common/Dlgcode.c @@ -13001,6 +13001,7 @@ BOOL CALLBACK SecurityTokenKeyfileDlgProc (HWND hwndDlg, UINT msg, WPARAM wParam finally_do ({ NormalCursor(); }); vector keyfileData; + finally_do_arg (vector *, &keyfileData, { if (!finally_arg->empty()) burn (&finally_arg->front(), finally_arg->size()); }); keyfile->GetKeyfileData (keyfileData); @@ -13011,8 +13012,6 @@ BOOL CALLBACK SecurityTokenKeyfileDlgProc (HWND hwndDlg, UINT msg, WPARAM wParam return 1; } - finally_do_arg (vector *, &keyfileData, { burn (&finally_arg->front(), finally_arg->size()); }); - if (!SaveBufferToFile ((char *) &keyfileData.front(), keyfilePath, (DWORD) keyfileData.size(), FALSE, FALSE)) throw SystemException (SRC_POS); } diff --git a/src/Common/EMVCard.cpp b/src/Common/EMVCard.cpp index 59dc3cf0..30f7f5f9 100644 --- a/src/Common/EMVCard.cpp +++ b/src/Common/EMVCard.cpp @@ -67,15 +67,18 @@ namespace VeraCrypt Clear(); } - EMVCard::EMVCard(const EMVCard& other) : - SCard(other), - m_aid(other.m_aid), - m_supportedAids(other.m_supportedAids), - m_iccCert(other.m_iccCert), - m_issuerCert(other.m_issuerCert), - m_cplcData(other.m_cplcData), - m_lastPANDigits(other.m_lastPANDigits) + EMVCard::EMVCard(const EMVCard& other) : SCard() { + try + { + *this = other; + } + catch (...) + { + // The destructor won't run if copying a later field fails. + Clear(); + throw; + } } EMVCard::EMVCard(EMVCard&& other) : @@ -93,6 +96,7 @@ namespace VeraCrypt { if (this != &other) { + Clear(); SCard::operator=(other); m_aid = other.m_aid; m_supportedAids = other.m_supportedAids; @@ -108,6 +112,7 @@ namespace VeraCrypt { if (this != &other) { + Clear(); SCard::operator=(other); m_reader = std::move(other.m_reader); m_aid = std::move(other.m_aid); @@ -124,9 +129,17 @@ namespace VeraCrypt { m_aid.clear(); m_supportedAids.clear(); + if (!m_iccCert.empty()) + burn(m_iccCert.data(), m_iccCert.size()); m_iccCert.clear(); + if (!m_issuerCert.empty()) + burn(m_issuerCert.data(), m_issuerCert.size()); m_issuerCert.clear(); + if (!m_cplcData.empty()) + burn(m_cplcData.data(), m_cplcData.size()); m_cplcData.clear(); + if (!m_lastPANDigits.empty()) + burn(&m_lastPANDigits[0], m_lastPANDigits.size() * sizeof(wchar_t)); m_lastPANDigits.clear(); } @@ -426,8 +439,14 @@ namespace VeraCrypt if (!issuerFound) throw EMVIssuerCertNotFound(); + if (!m_iccCert.empty()) + burn(m_iccCert.data(), m_iccCert.size()); m_iccCert = iccCert; + if (!m_issuerCert.empty()) + burn(m_issuerCert.data(), m_issuerCert.size()); m_issuerCert = issuerCert; + if (!m_cplcData.empty()) + burn(m_cplcData.data(), m_cplcData.size()); m_cplcData = cplcData; } diff --git a/src/Common/EMVToken.cpp b/src/Common/EMVToken.cpp index 8f89cb7e..5f23ca4b 100644 --- a/src/Common/EMVToken.cpp +++ b/src/Common/EMVToken.cpp @@ -79,10 +79,15 @@ namespace VeraCrypt map >::iterator emvCardsIt; shared_ptr card; vector iccCert; + finally_do_arg (vector *, &iccCert, { if (!finally_arg->empty()) burn (finally_arg->data(), finally_arg->size()); }); vector issuerCert; + finally_do_arg (vector *, &issuerCert, { if (!finally_arg->empty()) burn (finally_arg->data(), finally_arg->size()); }); vector cplcData; + finally_do_arg (vector *, &cplcData, { if (!finally_arg->empty()) burn (finally_arg->data(), finally_arg->size()); }); bool addNewCard = true; + if (!keyfileData.empty()) + burn (keyfileData.data(), keyfileData.size()); keyfileData.clear(); emvCardsIt = EMVToken::EMVCards.find(Token->SlotId); @@ -113,6 +118,16 @@ namespace VeraCrypt EMVToken::EMVCards.insert(make_pair(Token->SlotId, card)); } + // Allocate before copying so vector growth cannot release keyfile material. + size_t totalSize = iccCert.size(); + if (issuerCert.size() > keyfileData.max_size() - totalSize) + throw ParameterIncorrect (SRC_POS); + totalSize += issuerCert.size(); + if (cplcData.size() > keyfileData.max_size() - totalSize) + throw ParameterIncorrect (SRC_POS); + totalSize += cplcData.size(); + keyfileData.reserve(totalSize); + AppendData(keyfileData, iccCert.data(), iccCert.size(), 0, iccCert.size()); AppendData(keyfileData, issuerCert.data(), issuerCert.size(), 0, issuerCert.size()); AppendData(keyfileData, cplcData.data(), cplcData.size(), 0, cplcData.size()); diff --git a/src/Common/Keyfiles.c b/src/Common/Keyfiles.c index a0ff8afe..4dec10bc 100644 --- a/src/Common/Keyfiles.c +++ b/src/Common/Keyfiles.c @@ -254,6 +254,7 @@ BOOL KeyFilesApply (HWND hwndDlg, Password *password, KeyFile *firstKeyFile, con { // Apply security token keyfile vector keyfileData; + finally_do_arg (vector *, &keyfileData, { if (!finally_arg->empty()) burn (&finally_arg->front(), finally_arg->size()); }); TokenKeyfilePath secPath (kf->FileName); Token::getTokenKeyfile (secPath)->GetKeyfileData (keyfileData); @@ -286,7 +287,6 @@ BOOL KeyFilesApply (HWND hwndDlg, Password *password, KeyFile *firstKeyFile, con break; } - burn (&keyfileData.front(), keyfileData.size()); continue; } } diff --git a/src/Common/SecurityToken.cpp b/src/Common/SecurityToken.cpp index 6e7c2e29..b06828a5 100644 --- a/src/Common/SecurityToken.cpp +++ b/src/Common/SecurityToken.cpp @@ -168,9 +168,9 @@ namespace VeraCrypt // Some tokens report success even if the new object was truncated to fit in the available memory vector objectData; + finally_do_arg(vector *, &objectData, { if (!finally_arg->empty()) burn(&finally_arg->front(), finally_arg->size()); }); GetObjectAttribute(slotId, keyfileHandle, CKA_VALUE, objectData); - finally_do_arg(vector *, &objectData, { if (!finally_arg->empty()) burn(&finally_arg->front(), finally_arg->size()); }); if (objectData.size() != keyfileData.size()) { diff --git a/src/Common/Tcdefs.h b/src/Common/Tcdefs.h index 6220252a..8cc5d654 100644 --- a/src/Common/Tcdefs.h +++ b/src/Common/Tcdefs.h @@ -373,7 +373,7 @@ typedef unsigned char BOOLEAN; #if defined(_WIN32) && !defined(_UEFI) #define burn(mem,size) do { volatile char *burnm = (volatile char *)(mem); size_t burnc = size; RtlSecureZeroMemory (mem, size); while (burnc--) *burnm++ = 0; } while (0) #else -#define burn(mem,size) do { volatile char *burnm = (volatile char *)(mem); int burnc = size; while (burnc--) *burnm++ = 0; } while (0) +#define burn(mem,size) do { volatile char *burnm = (volatile char *)(mem); size_t burnc = size; while (burnc--) *burnm++ = 0; } while (0) #endif #define volatile_memcpy(d,s,size) do { volatile char *destm = (volatile char *)(d); volatile char *srcm = (volatile char *)(s); size_t memc = size; while (memc--) *destm++ = *srcm++; } while (0) diff --git a/src/Volume/Keyfile.cpp b/src/Volume/Keyfile.cpp index c5168ebc..8c148a21 100644 --- a/src/Volume/Keyfile.cpp +++ b/src/Volume/Keyfile.cpp @@ -11,6 +11,7 @@ */ #include "Platform/Serializer.h" +#include "Platform/Finally.h" #include "Common/SecurityToken.h" #include "Common/EMVToken.h" #include "Crc32.h" @@ -36,6 +37,7 @@ namespace VeraCrypt { // Apply keyfile generated by a security token vector keyfileData; + finally_do_arg (vector *, &keyfileData, { if (!finally_arg->empty()) burn (&finally_arg->front(), finally_arg->size()); }); Token::getTokenKeyfile(wstring(Path))->GetKeyfileData(keyfileData); if (keyfileData.size() < MinProcessedLength) @@ -57,8 +59,6 @@ namespace VeraCrypt break; } - - burn(&keyfileData.front(), keyfileData.size()); goto done; }