Bug 1545355: Uppercase hash tags before calling WinVerifyTrust on catalog files; r=mhowell
authorAaron Klotz <aklotz@mozilla.com>
Tue, 07 May 2019 16:58:24 +0000
changeset 531747 bfc71363efc998d502e494acdb4c43401a3da008
parent 531746 eea0648e7d3083ba14021578288c620b611e07fe
child 531748 1a940d2db42b486992125255c82fcd9a58e853f0
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmhowell
bugs1545355
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1545355: Uppercase hash tags before calling WinVerifyTrust on catalog files; r=mhowell On Windows 7, WinVerifyTrust fails unless the tag is uppercased. This patch also adds a missing call to CryptCATAdminReleaseCatalogContext, the need for which was poorly documented on MSDN. Differential Revision: https://phabricator.services.mozilla.com/D30146
mozglue/build/Authenticode.cpp
--- a/mozglue/build/Authenticode.cpp
+++ b/mozglue/build/Authenticode.cpp
@@ -26,27 +26,30 @@
 #  define NTDDI_VERSION NTDDI_WIN8
 #endif  // defined(NTDDI_VERSION)
 
 #include "Authenticode.h"
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/DynamicallyLinkedFunctionPtr.h"
+#include "mozilla/ScopeExit.h"
 #include "mozilla/TypeTraits.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/WindowsVersion.h"
 #include "nsWindowsHelpers.h"
 
 #include <windows.h>
 #include <softpub.h>
 #include <wincrypt.h>
 #include <wintrust.h>
 #include <mscat.h>
 
+#include <string.h>
+
 namespace {
 
 struct CertStoreDeleter {
   typedef HCERTSTORE pointer;
   void operator()(pointer aStore) { ::CertCloseStore(aStore, 0); }
 };
 
 struct CryptMsgDeleter {
@@ -302,22 +305,37 @@ bool SignedBinary::VerifySignature(const
   static const mozilla::DynamicallyLinkedFunctionPtr<decltype(
       &::CryptCATAdminEnumCatalogFromHash)>
       pCryptCATAdminEnumCatalogFromHash(L"wintrust.dll",
                                         "CryptCATAdminEnumCatalogFromHash");
   if (!pCryptCATAdminEnumCatalogFromHash) {
     return false;
   }
 
+  static const mozilla::DynamicallyLinkedFunctionPtr<decltype(
+      &::CryptCATAdminReleaseCatalogContext)>
+    pCryptCATAdminReleaseCatalogContext(L"wintrust.dll",
+                                        "CryptCATAdminReleaseCatalogContext");
+  if (!pCryptCATAdminReleaseCatalogContext) {
+    return false;
+  }
+
   HCATINFO catInfoHdl = pCryptCATAdminEnumCatalogFromHash(
       rawCatAdmin, hashBuf.get(), hashLen, 0, nullptr);
   if (!catInfoHdl) {
     return false;
   }
 
+  // We can't use UniquePtr for this because the deleter function requires two
+  // parameters.
+  auto cleanCatInfoHdl = mozilla::MakeScopeExit(
+    [rawCatAdmin, catInfoHdl]() -> void {
+    pCryptCATAdminReleaseCatalogContext(rawCatAdmin, catInfoHdl, 0);
+  });
+
   // We found a catalog! Now query for the path to the catalog file.
 
   static const mozilla::DynamicallyLinkedFunctionPtr<decltype(
       &::CryptCATCatalogInfoFromContext)>
       pCryptCATCatalogInfoFromContext(L"wintrust.dll",
                                       "CryptCATCatalogInfoFromContext");
   if (!pCryptCATCatalogInfoFromContext) {
     return false;
@@ -334,16 +352,23 @@ bool SignedBinary::VerifySignature(const
   DWORD strHashBufLen = (hashLen * 2) + 1;
   auto strHashBuf = mozilla::MakeUnique<wchar_t[]>(strHashBufLen);
   if (!::CryptBinaryToStringW(hashBuf.get(), hashLen,
                               CRYPT_STRING_HEXRAW | CRYPT_STRING_NOCRLF,
                               strHashBuf.get(), &strHashBufLen)) {
     return false;
   }
 
+  // Ensure that the tag is uppercase for WinVerifyTrust
+  // NB: CryptBinaryToStringW overwrites strHashBufLen with the length excluding
+  //     the null terminator, so we need to add it back for this call.
+  if (_wcsupr_s(strHashBuf.get(), strHashBufLen + 1)) {
+    return false;
+  }
+
   // Now, given the path to the catalog, and the path to the member (ie, the
   // binary whose hash we are validating), we may now validate. If the
   // validation is successful, we then QueryObject on the *catalog file*
   // instead of the binary.
 
   WINTRUST_CATALOG_INFO wtCatInfo = {sizeof(wtCatInfo)};
   wtCatInfo.pcwszCatalogFilePath = catInfo.wszCatalogFile;
   wtCatInfo.pcwszMemberTag = strHashBuf.get();