CVE-2023-40012: Improper Verification of Cryptographic Signature (CWE-347)
value: uthenticode is a small cross-platform library for partially verifying Authenticode digital signatures. Versions of uthenticode prior to the 2.x series did not check Extended Key Usages in certificates, in violation of the Authenticode X.509 certificate profile. As a result, a malicious user could produce a "signed" PE file that uthenticode would verify and consider valid using an X.509 certificate that isn't entitled to produce code signatures (e.g., a SSL certificate). By design, uthenticode does not perform full-chain validation. However, the absence of EKU validation was an unintended oversight. The 2.0.0 release series includes EKU checks. There are no workarounds to this vulnerability.
CWE description: The product does not verify, or incorrectly verifies, the cryptographic signature for data.
CVSS v3 base score: 7.5
Published: 2023-08-09T16:15Z
Repository: https://github.com/trailofbits/uthenticode
Language: C++
Files changed: README.md, src/CMakeLists.txt, src/uthenticode.cpp, test/helpers.h, test/signeddata-test.cpp
Commit date: 2023-07-28 16:44:36 -0400
Commit message: src: bump OpenSSL, EKU check (#78)

* src: bump OpenSSL, EKU check

Signed-off-by: William Woodruff <william@trailofbits.com>

* README: bump OpenSSL

Signed-off-by: William Woodruff <william@trailofbits.com>

* test: EKU test, testcase

Signed-off-by: William Woodruff <william@trailofbits.com>

---------

Signed-off-by: William Woodruff <william@trailofbits.com>
Diff: diff --git a/README.md b/README.md
index b45f0dd..c4d2c03 100644
--- a/README.md
+++ b/README.md
@@ -47,11 +47,11 @@ perspective will run on an unmodified Windows system. We make no claim that
 complete.
 
 ## Building
 
 *μthenticode* depends on [pe-parse](https://github.com/trailofbits/pe-parse)
-and OpenSSL 1.1.0, which are installed via `vcpkg` by following these steps:
+and OpenSSL 3.0 or higher, which are installed via `vcpkg` by following these steps:
 
 ```bash
 cmake -B build -S . -DCMAKE_TOOLCHAIN_FILE=<vcpkg-path>/scripts/buildsystems/vcpkg.cmake
 cmake --build build
 # the default install prefix is the build directory;
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 937d6ed..90f871d 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -2,11 +2,11 @@ cmake_minimum_required(VERSION 3.10 FATAL_ERROR)
 
 project(uthenticode)
 
 find_package(pe-parse REQUIRED)
 find_package(
-  OpenSSL 1.1
+  OpenSSL 3.0
   COMPONENTS Crypto
   REQUIRED
 )
 
 add_library("${PROJECT_NAME}" uthenticode.cpp)
diff --git a/src/uthenticode.cpp b/src/uthenticode.cpp
index 1efbb3e..dcca2b8 100644
--- a/src/uthenticode.cpp
+++ b/src/uthenticode.cpp
@@ -1,10 +1,11 @@
 #include "uthenticode.h"
 
 #include <openssl/crypto.h>
 #include <openssl/err.h>
 #include <openssl/x509.h>
+#include <openssl/x509v3.h>
 
 #include <algorithm>
 #include <array>
 #include <cstring>
 #include <iomanip>
@@ -207,10 +208,36 @@ bool SignedData::verify_signature() const {
 
   if (certs == nullptr) {
     return false;
   }
 
+  auto *signers_stack_ptr = PKCS7_get0_signers(p7_, nullptr, 0);
+  if (signers_stack_ptr == nullptr) {
+    return false;
+  }
+  auto signers_stack = impl::STACK_OF_X509_ptr(signers_stack_ptr, impl::SK_X509_free);
+
+  /* NOTE(ww): Authenticode specification, page 13: the signer must have the
+   * codeSigning EKU, **or** no member of the signer's chain may have it.
+   *
+   * The check below is more strict than that: **every** signer must have
+   * the codeSigning EKU, and we don't check the embedded chain (since
+   * we can't do full chain verification anyways).
+   */
+  for (auto i = 0; i < sk_X509_num(signers_stack.get()); ++i) {
+    auto *signer = sk_X509_value(signers_stack.get(), i);
+
+    /* NOTE(ww): Ths should really be X509_check_purpose with
+     * X509_PURPOSE_CODE_SIGN, but this is inexplicably not present
+     * in even the latest releases of OpenSSL as of 2023-05.
+     */
+    auto xku_flags = X509_get_extended_key_usage(signer);
+    if (!(xku_flags & XKU_CODE_SIGN)) {
+      return false;
+    }
+  }
+
   /* NOTE(ww): What happens below is a bit dumb: we convert our SpcIndirectDataContent back
    * into DER form so that we can unwrap its ASN.1 sequence and pass the underlying data
    * to PKCS7_verify for verification. This displays our intent a little more clearly than
    * our previous approach, which was to walk the PKCS#7 structure manually.
    */
diff --git a/test/assets/A_SSLippery_Slope.exe b/test/assets/A_SSLippery_Slope.exe
new file mode 100644
index 0000000..6e237d5
Binary files /dev/null and b/test/assets/A_SSLippery_Slope.exe differ
diff --git a/test/helpers.h b/test/helpers.h
index f48b9fe..a4da600 100644
--- a/test/helpers.h
+++ b/test/helpers.h
@@ -93,5 +93,21 @@ class AuthNest32PlusTest : public ::testing::Test {
     peparse::DestructParsedPE(pe);
   }
 
   peparse::parsed_pe *pe{nullptr};
 };
+
+class MissingEKUTest : public ::testing::Test {
+ protected:
+  void SetUp() override {
+    auto *file = UTHENTICODE_TEST_ASSETS "/A_SSLippery_Slope.exe";
+
+    pe = peparse::ParsePEFromFile(file);
+    ASSERT_TRUE(pe != nullptr);
+  }
+
+  void TearDown() override {
+    peparse::DestructParsedPE(pe);
+  }
+
+  peparse::parsed_pe *pe{nullptr};
+};
diff --git a/test/signeddata-test.cpp b/test/signeddata-test.cpp
index 2f038ca..4a6dd7c 100644
--- a/test/signeddata-test.cpp
+++ b/test/signeddata-test.cpp
@@ -114,5 +114,12 @@ TEST_F(AuthNest32PlusTest, SignedData_properties_nested) {
   auto const checksumstr = std::get<std::string>(checksum);
   ASSERT_EQ(checksumstr.size(), 64);
   ASSERT_STRCASEEQ(checksumstr.c_str(),
                    "ddc5b39c4292120745eb86a67eaa331032cc05a0dafaf6e28ec9aa0f189c408d");
 }
+
+TEST_F(MissingEKUTest, SignedData_missing_codesigning_EKU) {
+  auto certs = uthenticode::read_certs(pe);
+  auto signed_data = certs[0].as_signed_data();
+
+  ASSERT_FALSE(signed_data->verify_signature());
+}

Vulnerable code: and OpenSSL 1.1.0, which are installed via `vcpkg` by following these steps:
  OpenSSL 1.1
Fixed code: and OpenSSL 3.0 or higher, which are installed via `vcpkg` by following these steps:
  OpenSSL 3.0
#include <openssl/x509v3.h>
  auto *signers_stack_ptr = PKCS7_get0_signers(p7_, nullptr, 0);
  if (signers_stack_ptr == nullptr) {
    return false;
  }
  auto signers_stack = impl::STACK_OF_X509_ptr(signers_stack_ptr, impl::SK_X509_free);

  /* NOTE(ww): Authenticode specification, page 13: the signer must have the
   * codeSigning EKU, **or** no member of the signer's chain may have it.
   *
   * The check below is more strict than that: **every** signer must have
   * the codeSigning EKU, and we don't check the embedded chain (since
   * we can't do full chain verification anyways).
   */
  for (auto i = 0; i < sk_X509_num(signers_stack.get()); ++i) {
    auto *signer = sk_X509_value(signers_stack.get(), i);

    /* NOTE(ww): Ths should really be X509_check_purpose with
     * X509_PURPOSE_CODE_SIGN, but this is inexplicably not present
     * in even the latest releases of OpenSSL as of 2023-05.
     */
    auto xku_flags = X509_get_extended_key_usage(signer);
    if (!(xku_flags & XKU_CODE_SIGN)) {
      return false;
    }
  }


class MissingEKUTest : public ::testing::Test {
 protected:
  void SetUp() override {
    auto *file = UTHENTICODE_TEST_ASSETS "/A_SSLippery_Slope.exe";

    pe = peparse::ParsePEFromFile(file);
    ASSERT_TRUE(pe != nullptr);
  }

  void TearDown() override {
    peparse::DestructParsedPE(pe);
  }

  peparse::parsed_pe *pe{nullptr};
};

TEST_F(MissingEKUTest, SignedData_missing_codesigning_EKU) {
  auto certs = uthenticode::read_certs(pe);
  auto signed_data = certs[0].as_signed_data();

  ASSERT_FALSE(signed_data->verify_signature());
}