CVE-2023-39969 (C++, published 2023-08-09T16:15Z)
Description: value: uthenticode is a small cross-platform library for partially verifying Authenticode digital signatures. Version 1.0.9 of uthenticode hashed the entire file rather than hashing sections by virtual address, in violation of the Authenticode specification. As a result, an attacker could modify code within a binary without changing its Authenticode hash, making it appear valid from uthenticode's perspective. Versions of uthenticode prior to 1.0.9 are not vulnerable to this attack, nor are versions in the 2.x series. By design, uthenticode does not perform full-chain validation. However, the malleability of signature verification introduced in 1.0.9 was an unintended oversight. The 2.x series addresses the vulnerability. Versions prior to 1.0.9 are also not vulnerable, but users are encouraged to upgrade rather than downgrade. There are no workarounds to this vulnerability.
Severity: nan (CVSS v3 base score: 9.8)
Weakness: CWE-347 \u2014 Improper Verification of Cryptographic Signature
The product does not verify, or incorrectly verifies, the cryptographic signature for data.
Repository: https://github.com/trailofbits/uthenticode
Files changed:
src/include/uthenticode.h
src/svcli/svcli.cpp
src/uthenticode.cpp
test/helpers.h
test/signeddata-test.cpp
test/uthenticode-test.cpp
Commit message: Fix hashing (#84)

* Revert "Fix hash calculation (#62)"

This reverts commit d16e9ec2b4d54c1333137ae34f011e9eb3e0b962.

* test: stuffed PE tests

Signed-off-by: William Woodruff <william@trailofbits.com>

* uthenticode-test: re-add failing hashes

These hashes are correct; this can't be merged until these tests pass.

Signed-off-by: William Woodruff <william@trailofbits.com>

* src, test: don't support checksums on unsigned exes

Signed-off-by: William Woodruff <william@trailofbits.com>

* test/assets: stuffing example

Signed-off-by: William Woodruff <william@trailofbits.com>

* uthenticode: fix hashing on trailing data

Signed-off-by: William Woodruff <william@trailofbits.com>

* uthenticode: safer buffer operations

Signed-off-by: William Woodruff <william@trailofbits.com>

* uthenticode: free trailer_buf once we're done with it

Signed-off-by: William Woodruff <william@trailofbits.com>

---------

Signed-off-by: William Woodruff <william@trailofbits.com>

Vulnerable code:
 * @return      the resulting digest, or an empty string on failure
std::string calculate_checksum(peparse::parsed_pe *pe, checksum_kind kind);
    std::cout << std::setw(6) << kind << ": " << uthenticode::calculate_checksum(pe, kind) << '\n';
std::string calculate_checksum(peparse::parsed_pe *pe, checksum_kind kind) {
  /* We'll stash the bits of the PE that we need to hash in this buffer.
   */
  std::vector<std::uint8_t> pe_bits;
  pe_bits.reserve(size_of_headers);

  /* Hash pe_bits, which contains the rest of the header.
   */
  const auto *md = EVP_get_digestbynid(nid);
  auto *md_ctx = EVP_MD_CTX_new();
  EVP_DigestInit(md_ctx, md);
  EVP_DigestUpdate(md_ctx, pe_bits.data(), pe_bits.size());
  if (security_dir.VirtualAddress > 0) {
    /* If a certificate table exists, hash everything before and after it.
     */
    EVP_DigestUpdate(md_ctx,
                     pe->fileBuffer->buf + size_of_headers,
                     security_dir.VirtualAddress - size_of_headers);
    /* Most PEs won't have any trailing data but the Authenticode specification is explicit about
     * hashing any if it exists.
     */
    EVP_DigestUpdate(md_ctx,
                     pe->fileBuffer->buf + security_dir.VirtualAddress + security_dir.Size,
                     pe->fileBuffer->bufLen - (security_dir.VirtualAddress + security_dir.Size));
  } else {
    /* If there's no certificate table, just hash the rest of the file.
     */
    EVP_DigestUpdate(
        md_ctx, pe->fileBuffer->buf + size_of_headers, pe->fileBuffer->bufLen - size_of_headers);
  /* Finally, finish hashing the damn thing.
  EXPECT_TRUE(unk.empty());
  EXPECT_EQ(md5.size(), 32);
  EXPECT_STRCASEEQ(md5.c_str(), "A31557B1E39554C88C69AAE1DFAAF314");
  EXPECT_EQ(sha1.size(), 40);
  EXPECT_STRCASEEQ(sha1.c_str(), "2B316F0552972605D509321F31F4274533C93161");
  EXPECT_EQ(sha256.size(), 64);
  EXPECT_STRCASEEQ(sha256.c_str(),
                   "6B7FA3E8298F33BC47F4ABB9C845930B1EACC0DAD96503CFA52D4EA18DDC89F0");
  EXPECT_TRUE(unk.empty());
  EXPECT_EQ(md5.size(), 32);
  EXPECT_STRCASEEQ(md5.c_str(), "64c29391b57679b2973ac562cf64685d");
  EXPECT_EQ(sha1.size(), 40);
  EXPECT_STRCASEEQ(sha1.c_str(), "6663dd7c24fa84fce7f16e0b02689952c06cfa22");
  EXPECT_EQ(sha256.size(), 64);
  EXPECT_STRCASEEQ(sha256.c_str(),
  EXPECT_TRUE(unk.empty());
  EXPECT_EQ(md5.size(), 32);
  EXPECT_STRCASEEQ(md5.c_str(), "64c29391b57679b2973ac562cf64685d");
  EXPECT_EQ(sha1.size(), 40);
  EXPECT_STRCASEEQ(sha1.c_str(), "6663dd7c24fa84fce7f16e0b02689952c06cfa22");
  EXPECT_EQ(sha256.size(), 64);
  EXPECT_STRCASEEQ(sha256.c_str(),
  EXPECT_TRUE(unk.empty());
  EXPECT_EQ(md5.size(), 32);
  EXPECT_STRCASEEQ(md5.c_str(), "ea0235b77d552633c5a38974cef0e2b5");
  EXPECT_EQ(sha1.size(), 40);
  EXPECT_STRCASEEQ(sha1.c_str(), "2559e91a60953a5e16f9650f5f88953a2cca5425");
  EXPECT_EQ(sha256.size(), 64);
  EXPECT_STRCASEEQ(sha256.c_str(),

Fixed code:
 * `parsed_pe` must contain a security directory; calculating the checksum
 * of a "bare" PE is not supported.
 *
 * @return      the resulting digest, or `std::nullopt` on failure
std::optional<std::string> calculate_checksum(peparse::parsed_pe *pe, checksum_kind kind);
    auto cksum = uthenticode::calculate_checksum(pe, kind);
    if (cksum.has_value()) {
      std::cout << std::setw(6) << kind << ": " << cksum.value() << '\n';
    } else {
      std::cout << std::setw(6) << kind << ": NONE (not signed)\n";
    }
std::optional<std::string> calculate_checksum(peparse::parsed_pe *pe, checksum_kind kind) {
  /* We'll stash the bits of the PE that we need to hash in this buffer.
   * Reserve the original PE's size upfront, since we expect the hashed data
   * to be only slightly smaller.
   */
  std::vector<std::uint8_t> pe_bits;
  pe_bits.reserve(pe->fileBuffer->bufLen);

  /* We explicitly don't support hashing files that don't contain a security
   * directory; not because we *can't*, but because doing so isn't well defined
   * in the Authenticode specification.
   */
  if (security_dir.VirtualAddress == 0) {
    return {};
  }

  struct iter_sec_ctx {
    impl::SectionList sections;
    uint32_t total_bytes_hashed;
  };
  impl::SectionList sections;
  uint32_t total_bytes_hashed = size_of_headers;
  iter_sec_ctx ctx = {sections, total_bytes_hashed};
  /* Build up the list of sections in the PE, in ascending order by PointerToRawData
   * (i.e., by file offset).
   *
   * NOTE(ww): Ideally we'd use a capture with the C++ lambda here, but C++ lambdas can't be
   * used within C callbacks unless they're captureless.
   */
  peparse::IterSec(
      pe,
      [](void *cbd,
         [[maybe_unused]] const peparse::VA &secBase,
         [[maybe_unused]] const std::string &sectionName,
         [[maybe_unused]] const peparse::image_section_header &sec,
         const peparse::bounded_buffer *b) -> int {
        auto &ctx = *static_cast<iter_sec_ctx *>(cbd);
        ctx.sections.emplace_back(b);
        ctx.total_bytes_hashed += sec.SizeOfRawData;
        return 0;
      },
      &ctx);

  /* Copy each section's data into pe_bits, in ascending order.
   */
  for (const auto &section : sections) {
    pe_bits.insert(pe_bits.end(), section->buf, section->buf + section->bufLen);
  /* Also copy any data that happens to be trailing the certificate table into pe_bits.
   * Most PEs won't have any trailing data but the Authenticode specification is explicit about
   * hashing any if it exists.
   */
  auto file_size = pe->fileBuffer->bufLen;
  auto extra_data_size = file_size - (security_dir.Size + total_bytes_hashed);
  auto *trailer_buf = peparse::splitBuffer(
      pe->fileBuffer, total_bytes_hashed, total_bytes_hashed + extra_data_size);
  if (trailer_buf == nullptr) {
    return {};
  }
  pe_bits.insert(pe_bits.end(), trailer_buf->buf, trailer_buf->buf + trailer_buf->bufLen);
  delete trailer_buf;

  /* Finally, hash the damn thing.
   *
   * NOTE(ww): Instead of building up pe_bits and hashing it in one pass, we
   * could hash it incrementally with each section. This would also solve
   * the capture problem with the C++ callback above and would reduce
   * the number of needed allocations.
  const auto *md = EVP_get_digestbynid(nid);
  auto *md_ctx = EVP_MD_CTX_new();

  EVP_DigestInit(md_ctx, md);
  EVP_DigestUpdate(md_ctx, pe_bits.data(), pe_bits.size());

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

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

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

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

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

  // The signature in this PE is valid, but it doesn't actually
  // match the PE's calculated checksum. See `StuffingTest.verify`
  // for the corresponding test.
  ASSERT_TRUE(signed_data->verify_signature());
}
  EXPECT_FALSE(unk.has_value());
  EXPECT_FALSE(md5.has_value());
  EXPECT_FALSE(sha1.has_value());
  EXPECT_FALSE(sha256.has_value());
  EXPECT_FALSE(unk.has_value());
  EXPECT_EQ(md5.value().size(), 32);
  EXPECT_STRCASEEQ(md5.value().c_str(), "64c29391b57679b2973ac562cf64685d");
  EXPECT_EQ(sha1.value().size(), 40);
  EXPECT_STRCASEEQ(sha1.value().c_str(), "6663dd7c24fa84fce7f16e0b02689952c06cfa22");
  EXPECT_EQ(sha256.value().size(), 64);
  EXPECT_STRCASEEQ(sha256.value().c_str(),
  EXPECT_FALSE(unk.has_value());
  EXPECT_EQ(md5.value().size(), 32);
  EXPECT_STRCASEEQ(md5.value().c_str(), "64c29391b57679b2973ac562cf64685d");
  EXPECT_EQ(sha1.value().size(), 40);
  EXPECT_STRCASEEQ(sha1.value().c_str(), "6663dd7c24fa84fce7f16e0b02689952c06cfa22");
  EXPECT_EQ(sha256.value().size(), 64);
  EXPECT_STRCASEEQ(sha256.value().c_str(),
  EXPECT_FALSE(unk.has_value());
  EXPECT_EQ(md5.value().size(), 32);
  EXPECT_STRCASEEQ(md5.value().c_str(), "ea0235b77d552633c5a38974cef0e2b5");
  EXPECT_EQ(sha1.value().size(), 40);
  EXPECT_STRCASEEQ(sha1.value().c_str(), "2559e91a60953a5e16f9650f5f88953a2cca5425");
  EXPECT_EQ(sha256.value().size(), 64);
  EXPECT_STRCASEEQ(sha256.value().c_str(),

TEST_F(MissingEKUTest, verify) {
  EXPECT_FALSE(uthenticode::verify(pe));
}

TEST_F(StuffingTest, verify) {
  EXPECT_FALSE(uthenticode::verify(pe));
}