Stop encrypting hidden variables.

They were persisted in plaintext, and always had to be decrypted and sent to a separate process to actually be used.

1 files changed, 22 insertions, 292 deletions

diff --git a/src/engine/variant.cpp b/src/engine/variant.cpp
index 82f465b2..2267ee7b 100644
--- a/src/engine/variant.cpp
+++ b/src/engine/variant.cpp
@@ -2,8 +2,6 @@
 
 #include "precomp.h"
 
-#define VARIANT_ENCRYPTION_SCOPE CRYPTPROTECTMEMORY_SAME_PROCESS
-
 // internal function declarations
 
 static HRESULT GetVersionInternal(
@@ -12,25 +10,6 @@ static HRESULT GetVersionInternal(
     __in BOOL fSilent,
     __out VERUTIL_VERSION** ppValue
     );
-static HRESULT BVariantEncryptString(
-    __in BURN_VARIANT* pVariant,
-    __in BOOL fEncrypt
-    );
-
-static void BVariantRetrieveNumeric(
-    __in BURN_VARIANT* pVariant,
-    __out LONGLONG* pllValue
-    );
-
-static HRESULT BVariantRetrieveDecryptedString(
-    __in BURN_VARIANT* pVariant,
-    __out LPWSTR* psczValue
-    );
-
-static void BVariantRetrieveVersion(
-    __in BURN_VARIANT* pVariant,
-    __out VERUTIL_VERSION** ppValue
-    );
 
 // function definitions
 
@@ -46,38 +25,28 @@ extern "C" void BVariantUninitialize(
     SecureZeroMemory(pVariant, sizeof(BURN_VARIANT));
 }
 
-// The contents of pllValue may be sensitive, should keep encrypted and SecureZeroMemory.
 extern "C" HRESULT BVariantGetNumeric(
     __in BURN_VARIANT* pVariant,
     __out LONGLONG* pllValue
     )
 {
     HRESULT hr = S_OK;
-    LPWSTR sczValue = NULL;
-    VERUTIL_VERSION* pVersionValue = NULL;
 
     switch (pVariant->Type)
     {
     case BURN_VARIANT_TYPE_NUMERIC:
-        BVariantRetrieveNumeric(pVariant, pllValue);
+        *pllValue = pVariant->llValue;
         break;
     case BURN_VARIANT_TYPE_FORMATTED: __fallthrough;
     case BURN_VARIANT_TYPE_STRING:
-        hr = BVariantRetrieveDecryptedString(pVariant, &sczValue);
-        if (SUCCEEDED(hr))
+        hr = StrStringToInt64(pVariant->sczValue, 0, pllValue);
+        if (FAILED(hr))
         {
-            hr = StrStringToInt64(sczValue, 0, pllValue);
-            if (FAILED(hr))
-            {
-                hr = DISP_E_TYPEMISMATCH;
-            }
+            hr = DISP_E_TYPEMISMATCH;
         }
-        StrSecureZeroFreeString(sczValue);
         break;
     case BURN_VARIANT_TYPE_VERSION:
-        BVariantRetrieveVersion(pVariant, &pVersionValue);
-
-        hr = StrStringToInt64(pVersionValue->sczVersion, 0, pllValue);
+        hr = StrStringToInt64(pVariant->pValue ? pVariant->pValue->sczVersion : NULL, 0, pllValue);
         if (FAILED(hr))
         {
             hr = DISP_E_TYPEMISMATCH;
@@ -91,35 +60,27 @@ extern "C" HRESULT BVariantGetNumeric(
     return hr;
 }
 
-// The contents of psczValue may be sensitive, should keep encrypted and SecureZeroFree.
 extern "C" HRESULT BVariantGetString(
     __in BURN_VARIANT* pVariant,
     __out_z LPWSTR* psczValue
     )
 {
     HRESULT hr = S_OK;
-    LONGLONG llValue = 0;
-    VERUTIL_VERSION* pVersionValue = NULL;
 
     switch (pVariant->Type)
     {
     case BURN_VARIANT_TYPE_NUMERIC:
-        BVariantRetrieveNumeric(pVariant, &llValue);
-        if (SUCCEEDED(hr))
-        {
-            hr = StrAllocFormattedSecure(psczValue, L"%I64d", llValue);
-            ExitOnFailure(hr, "Failed to convert int64 to string.");
-        }
-        SecureZeroMemory(&llValue, sizeof(llValue));
+        hr = StrAllocFormattedSecure(psczValue, L"%I64d", pVariant->llValue);
+        ExitOnFailure(hr, "Failed to convert int64 to string.");
         break;
     case BURN_VARIANT_TYPE_FORMATTED: __fallthrough;
     case BURN_VARIANT_TYPE_STRING:
-        hr = BVariantRetrieveDecryptedString(pVariant, psczValue);
+        hr = StrAllocStringSecure(psczValue, pVariant->sczValue, 0);
+        ExitOnFailure(hr, "Failed to copy string value.");
         break;
     case BURN_VARIANT_TYPE_VERSION:
-        BVariantRetrieveVersion(pVariant, &pVersionValue);
-
-        hr = StrAllocStringSecure(psczValue, pVersionValue->sczVersion, 0);
+        hr = StrAllocStringSecure(psczValue, pVariant->pValue ? pVariant->pValue->sczVersion : NULL, 0);
+        ExitOnFailure(hr, "Failed to copy version value.");
         break;
     default:
         hr = E_INVALIDARG;
@@ -130,7 +91,6 @@ LExit:
     return hr;
 }
 
-// The contents of ppValue may be sensitive, should keep encrypted and SecureZeroMemory.
 extern "C" HRESULT BVariantGetVersion(
     __in BURN_VARIANT* pVariant,
     __out VERUTIL_VERSION** ppValue
@@ -139,7 +99,6 @@ extern "C" HRESULT BVariantGetVersion(
     return GetVersionInternal(pVariant, FALSE, FALSE, ppValue);
 }
 
-// The contents of ppValue may be sensitive, should keep encrypted and SecureZeroMemory.
 extern "C" HRESULT BVariantGetVersionHidden(
     __in BURN_VARIANT* pVariant,
     __in BOOL fHidden,
@@ -149,7 +108,6 @@ extern "C" HRESULT BVariantGetVersionHidden(
     return GetVersionInternal(pVariant, fHidden, FALSE, ppValue);
 }
 
-// The contents of ppValue may be sensitive, should keep encrypted and SecureZeroMemory.
 extern "C" HRESULT BVariantGetVersionSilent(
     __in BURN_VARIANT* pVariant,
     __in BOOL fSilent,
@@ -167,44 +125,28 @@ static HRESULT GetVersionInternal(
     )
 {
     HRESULT hr = S_OK;
-    LONGLONG llValue = 0;
-    LPWSTR sczValue = NULL;
-    VERUTIL_VERSION* pValue = NULL;
 
     switch (pVariant->Type)
     {
     case BURN_VARIANT_TYPE_NUMERIC:
-        BVariantRetrieveNumeric(pVariant, &llValue);
-
-        hr = VerVersionFromQword(llValue, ppValue);
+        hr = VerVersionFromQword(pVariant->llValue, ppValue);
         break;
     case BURN_VARIANT_TYPE_FORMATTED: __fallthrough;
     case BURN_VARIANT_TYPE_STRING:
-        hr = BVariantRetrieveDecryptedString(pVariant, &sczValue);
-        if (SUCCEEDED(hr))
+        hr = VerParseVersion(pVariant->sczValue, 0, FALSE, ppValue);
+        if (SUCCEEDED(hr) && !fSilent && (*ppValue)->fInvalid)
         {
-            hr = VerParseVersion(sczValue, 0, FALSE, ppValue);
-            if (FAILED(hr))
-            {
-                hr = DISP_E_TYPEMISMATCH;
-            }
-            else if (!fSilent && (*ppValue)->fInvalid)
-            {
-                LogId(REPORT_WARNING, MSG_INVALID_VERSION_COERSION, fHidden ? L"*****" : sczValue);
-            }
+            LogId(REPORT_WARNING, MSG_INVALID_VERSION_COERSION, fHidden ? L"*****" : pVariant->sczValue);
         }
-        StrSecureZeroFreeString(sczValue);
         break;
     case BURN_VARIANT_TYPE_VERSION:
-        BVariantRetrieveVersion(pVariant, &pValue);
-
-        if (!pValue)
+        if (!pVariant->pValue)
         {
             *ppValue = NULL;
         }
         else
         {
-            hr = VerCopyVersion(pValue, ppValue);
+            hr = VerCopyVersion(pVariant->pValue, ppValue);
         }
         break;
     default:
@@ -221,7 +163,6 @@ extern "C" HRESULT BVariantSetNumeric(
     )
 {
     HRESULT hr = S_OK;
-    BOOL fEncrypt = pVariant->fEncryptString;
 
     if (BURN_VARIANT_TYPE_FORMATTED == pVariant->Type ||
         BURN_VARIANT_TYPE_STRING == pVariant->Type)
@@ -231,7 +172,6 @@ extern "C" HRESULT BVariantSetNumeric(
     memset(pVariant, 0, sizeof(BURN_VARIANT));
     pVariant->llValue = llValue;
     pVariant->Type = BURN_VARIANT_TYPE_NUMERIC;
-    BVariantSetEncryption(pVariant, fEncrypt);
 
     return hr;
 }
@@ -244,7 +184,6 @@ extern "C" HRESULT BVariantSetString(
     )
 {
     HRESULT hr = S_OK;
-    BOOL fEncrypt = pVariant->fEncryptString;
 
     if (!wzValue) // if we're nulling out the string, make the variable NONE.
     {
@@ -257,11 +196,6 @@ extern "C" HRESULT BVariantSetString(
         {
             memset(pVariant, 0, sizeof(BURN_VARIANT));
         }
-        else
-        {
-            // We're about to copy an unencrypted value.
-            pVariant->fEncryptString = FALSE;
-        }
 
         hr = StrAllocStringSecure(&pVariant->sczValue, wzValue, cchValue);
         ExitOnFailure(hr, "Failed to copy string.");
@@ -270,7 +204,6 @@ extern "C" HRESULT BVariantSetString(
     }
 
 LExit:
-    BVariantSetEncryption(pVariant, fEncrypt);
     return hr;
 }
 
@@ -280,7 +213,6 @@ extern "C" HRESULT BVariantSetVersion(
     )
 {
     HRESULT hr = S_OK;
-    BOOL fEncryptValue = pVariant->fEncryptString;
 
     if (!pValue) // if we're nulling out the version, make the variable NONE.
     {
@@ -298,8 +230,6 @@ extern "C" HRESULT BVariantSetVersion(
         pVariant->Type = BURN_VARIANT_TYPE_VERSION;
     }
 
-    BVariantSetEncryption(pVariant, fEncryptValue);
-
     return hr;
 }
 
@@ -309,10 +239,6 @@ extern "C" HRESULT BVariantSetValue(
     )
 {
     HRESULT hr = S_OK;
-    LONGLONG llValue = 0;
-    LPWSTR sczValue = NULL;
-    VERUTIL_VERSION* pVersionValue = NULL;
-    BOOL fEncrypt = pVariant->fEncryptString;
 
     switch (pValue->Type)
     {
@@ -320,35 +246,19 @@ extern "C" HRESULT BVariantSetValue(
         BVariantUninitialize(pVariant);
         break;
     case BURN_VARIANT_TYPE_NUMERIC:
-        hr = BVariantGetNumeric(pValue, &llValue);
-        if (SUCCEEDED(hr))
-        {
-            hr = BVariantSetNumeric(pVariant, llValue);
-        }
-        SecureZeroMemory(&llValue, sizeof(llValue));
+        hr = BVariantSetNumeric(pVariant, pValue->llValue);
         break;
     case BURN_VARIANT_TYPE_FORMATTED: __fallthrough;
     case BURN_VARIANT_TYPE_STRING:
-        hr = BVariantGetString(pValue, &sczValue);
-        if (SUCCEEDED(hr))
-        {
-            hr = BVariantSetString(pVariant, sczValue, 0, BURN_VARIANT_TYPE_FORMATTED == pValue->Type);
-        }
-        StrSecureZeroFreeString(sczValue);
+        hr = BVariantSetString(pVariant, pValue->sczValue, 0, BURN_VARIANT_TYPE_FORMATTED == pValue->Type);
         break;
     case BURN_VARIANT_TYPE_VERSION:
-        hr = BVariantGetVersionSilent(pValue, TRUE, &pVersionValue);
-        if (SUCCEEDED(hr))
-        {
-            hr = BVariantSetVersion(pVariant, pVersionValue);
-        }
+        hr = BVariantSetVersion(pVariant, pValue->pValue);
         break;
     default:
         hr = E_INVALIDARG;
     }
-    ExitOnFailure(hr, "Failed to copy variant.");
-
-    hr = BVariantSetEncryption(pVariant, fEncrypt);
+    ExitOnFailure(hr, "Failed to copy variant value.");
 
 LExit:
     return hr;
@@ -359,50 +269,7 @@ extern "C" HRESULT BVariantCopy(
     __out BURN_VARIANT* pTarget
     )
 {
-    HRESULT hr = S_OK;
-    LONGLONG llValue = 0;
-    LPWSTR sczValue = NULL;
-    VERUTIL_VERSION* pVersionValue = 0;
-
-    BVariantUninitialize(pTarget);
-
-    switch (pSource->Type)
-    {
-    case BURN_VARIANT_TYPE_NONE:
-        break;
-    case BURN_VARIANT_TYPE_NUMERIC:
-        hr = BVariantGetNumeric(pSource, &llValue);
-        if (SUCCEEDED(hr))
-        {
-            hr = BVariantSetNumeric(pTarget, llValue);
-        }
-        SecureZeroMemory(&llValue, sizeof(llValue));
-        break;
-    case BURN_VARIANT_TYPE_FORMATTED: __fallthrough;
-    case BURN_VARIANT_TYPE_STRING:
-        hr = BVariantGetString(pSource, &sczValue);
-        if (SUCCEEDED(hr))
-        {
-            hr = BVariantSetString(pTarget, sczValue, 0, BURN_VARIANT_TYPE_FORMATTED == pSource->Type);
-        }
-        StrSecureZeroFreeString(sczValue);
-        break;
-    case BURN_VARIANT_TYPE_VERSION:
-        hr = BVariantGetVersionSilent(pSource, TRUE, &pVersionValue);
-        if (SUCCEEDED(hr))
-        {
-            hr = BVariantSetVersion(pTarget, pVersionValue);
-        }
-        break;
-    default:
-        hr = E_INVALIDARG;
-    }
-    ExitOnFailure(hr, "Failed to copy variant.");
-
-    hr = BVariantSetEncryption(pTarget, pSource->fEncryptString);
-
-LExit:
-    return hr;
+    return BVariantSetValue(pTarget, pSource);
 }
 
 extern "C" HRESULT BVariantChangeType(
@@ -412,7 +279,6 @@ extern "C" HRESULT BVariantChangeType(
 {
     HRESULT hr = S_OK;
     BURN_VARIANT variant = { };
-    BOOL fEncrypt = pVariant->fEncryptString;
 
     if (pVariant->Type == type)
     {
@@ -449,143 +315,7 @@ extern "C" HRESULT BVariantChangeType(
     BVariantUninitialize(pVariant);
     memcpy_s(pVariant, sizeof(BURN_VARIANT), &variant, sizeof(BURN_VARIANT));
     SecureZeroMemory(&variant, sizeof(BURN_VARIANT));
-    BVariantSetEncryption(pVariant, fEncrypt);
 
 LExit:
     return hr;
 }
-
-extern "C" HRESULT BVariantSetEncryption(
-    __in BURN_VARIANT* pVariant,
-    __in BOOL fEncrypt
-    )
-{
-    HRESULT hr = S_OK;
-
-    if (pVariant->fEncryptString == fEncrypt)
-    {
-        // The requested encryption state is already applied.
-        ExitFunction();
-    }
-    
-    switch (pVariant->Type)
-    {
-    case BURN_VARIANT_TYPE_NONE:
-    case BURN_VARIANT_TYPE_NUMERIC:
-    case BURN_VARIANT_TYPE_VERSION:
-        hr = S_OK;
-        break;
-    case BURN_VARIANT_TYPE_FORMATTED: __fallthrough;
-    case BURN_VARIANT_TYPE_STRING:
-        hr = BVariantEncryptString(pVariant, fEncrypt);
-        break;
-    default:
-        hr = E_INVALIDARG;
-    }
-    ExitOnFailure(hr, "Failed to set the variant's encryption state");
-    pVariant->fEncryptString = fEncrypt;
-
-LExit:
-    return hr;
-}
-
-static HRESULT BVariantEncryptString(
-    __in BURN_VARIANT* pVariant,
-    __in BOOL fEncrypt
-    )
-{
-    HRESULT hr = S_OK;
-    SIZE_T cbData = 0;
-
-    if (NULL == pVariant->sczValue)
-    {
-        ExitFunction();
-    }
-
-    cbData = MemSize(pVariant->sczValue);
-    if (-1 == cbData)
-    {
-        hr = E_INVALIDARG;
-        ExitOnFailure(hr, "Failed to get the size of the string");
-    }
-    
-    DWORD remainder = fEncrypt ? cbData % CRYP_ENCRYPT_MEMORY_SIZE : 0;
-    DWORD extraNeeded = 0 < remainder ? CRYP_ENCRYPT_MEMORY_SIZE - remainder : 0;
-    if ((MAXDWORD - extraNeeded) < cbData)
-    {
-        hr = E_INVALIDDATA;
-        ExitOnFailure(hr, "The string is too big: size %u", cbData);
-    }
-    else if (0 < extraNeeded)
-    {
-        cbData += extraNeeded;
-        LPVOID pvNew = NULL;
-        hr = MemReAllocSecure(static_cast<LPVOID>(pVariant->sczValue), cbData, TRUE, &pvNew);
-        ExitOnFailure(hr, "Failed to resize the string so it could be encrypted");
-        pVariant->sczValue = static_cast<LPWSTR>(pvNew);
-    }
-    
-    if (fEncrypt)
-    {
-        hr = CrypEncryptMemory(pVariant->sczValue, static_cast<DWORD>(cbData), VARIANT_ENCRYPTION_SCOPE);
-    }
-    else
-    {
-        hr = CrypDecryptMemory(pVariant->sczValue, static_cast<DWORD>(cbData), VARIANT_ENCRYPTION_SCOPE);
-    }
-
-LExit:
-    return hr;
-}
-
-static void BVariantRetrieveNumeric(
-    __in BURN_VARIANT* pVariant,
-    __out LONGLONG* pllValue
-    )
-{
-    Assert(NULL != pllValue);
-
-    *pllValue = pVariant->llValue;
-}
-
-// The contents of psczValue may be sensitive, should keep encrypted and SecureZeroFree.
-static HRESULT BVariantRetrieveDecryptedString(
-    __in BURN_VARIANT* pVariant,
-    __out LPWSTR* psczValue
-    )
-{
-    HRESULT hr = S_OK;
-
-    if (!pVariant->sczValue)
-    {
-        *psczValue = NULL;
-        ExitFunction();
-    }
-
-    if (pVariant->fEncryptString)
-    {
-        hr = BVariantEncryptString(pVariant, FALSE);
-        ExitOnFailure(hr, "Failed to decrypt string");
-    }
-
-    hr = StrAllocStringSecure(psczValue, pVariant->sczValue, 0);
-    ExitOnFailure(hr, "Failed to copy value.");
-
-    if (pVariant->fEncryptString)
-    {
-        hr = BVariantEncryptString(pVariant, TRUE);
-    }
-
-LExit:
-    return hr;
-}
-
-static void BVariantRetrieveVersion(
-    __in BURN_VARIANT* pVariant,
-    __out VERUTIL_VERSION** ppValue
-    )
-{
-    Assert(ppValue);
-
-    *ppValue = pVariant->pValue;
-}