trailofbits / uthenticode

A cross-platform library for verifying Authenticode signatures
https://trailofbits.github.io/uthenticode/
MIT License
133 stars 33 forks source link

Wrong hash calculated for some PE files #59

Closed pkdawson closed 2 years ago

pkdawson commented 2 years ago

I've found that some files - especially installers - fail to verify because the calculated hash doesn't match.

Here's the latest Firefox installer, for example: https://ftp.mozilla.org/pub/firefox/releases/101.0/win64/en-US/Firefox%20Setup%20101.0.exe

Its embedded SHA-256 hash is DEF2F92C51E7DD222F480B9459C1E2E0A35C598453FC85EF74D9439D105AC853, but uthenticode::calculate_checksum returns 9338FBD1F37B5544CD759681C23A8343CBE4043CDB3A89A1818C909C81AFC6AC.

The problem is that pe_bits only contains 134,644 bytes at the time it's hashed, a tiny fraction of the EXE's total 56,014,584 bytes. It seems that enumerating the PE sections is not a reliable way to collect the data we need to hash, regardless of what Microsoft's documentation says.

The simpler approach of just hashing the file in order, skipping a few pieces, gives the correct result. For example, PeNet (Apache licensed) has an implementation like this: https://github.com/secana/PeNet/blob/ec0bb60d62c9d44dc7d5648ac742f96de3a0a166/src/PeNet/Header/Authenticode/AuthenticodeInfo.cs#L155

If you want, I'd be happy to submit a PR along those lines, since it's work I need to do anyway.

(I'd also like to get uthenticode working with .msi and .cat files, but that's a separate issue)

woodruffw commented 2 years ago

Thanks for the report!

Yes, a PR would be greatly appreciated. I'd also be happy to review changes for MSI and CAT files in the future.

hugmyndakassi commented 1 year ago

(I'd also like to get uthenticode working with .msi and .cat files, but that's a separate issue)

Why not throw in .cab files while at it? 😜

mtrojnar/osslsigncode has an msi.c and refers to microsoft/compoundfilereader for the heavy lifting of the compound file format. Since it's also MIT license, perhaps it's something that could be included as a compile time option all the while relying on the code from MS?

PS: for checking validity of assumptions etc, it's also worth looking into avast/authenticode-parser at times. It's been only around since 2021 and clearly has a different focus; and it's in C instead of C++. But it's also liberally licensed like µthenticode.

woodruffw commented 1 year ago

CAB support would also be welcome 🙂

Since it's also MIT license, perhaps it's something that could be included as a compile time option all the while relying on the code from MS?

Given that it's MIT licensed, I'd be okay with us vendoring it here, assuming it doesn't cause any regressions with our current ASan/valgrind/etc. checks in the testsuite.

rul3rst4 commented 1 year ago

I'm interested in making it work with .cat files. Does any of you know some documentation where I can start with?

woodruffw commented 1 year ago

I'm only familiar with the stuff MS has here: https://learn.microsoft.com/en-us/windows-hardware/drivers/install/catalog-files

It sounds like they're essentially a detached signature over a bunch of digests of individual files. So the verification process should be pretty similar to what's already in the codebase, but with a detached signature rather than one baked into the PKCS#7 blob.