search

trailofbits / uthenticode

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

Fix hashing #84

Closed woodruffw closed 1 year ago

woodruffw commented 1 year ago

This reverts commit d16e9ec2b4d54c1333137ae34f011e9eb3e0b962.

CC @pkdawson: This is going to revert your work in #62 due to some issues around Authenticode stuffing that were recently brought to our attention.

My plan is to re-merge your fix, tweaked to perform the section sorting the way we originally did it.

woodruffw commented 1 year ago

Marking as draft until I fix the hash calculation for unsigned binaries.

woodruffw commented 1 year ago

Marking as draft until I fix the hash calculation for unsigned binaries.

I'm just going to punt on this entirely and update the API to not support hashing unsigned binaries: doing so isn't well defined in the Authenticode spec anyways.

pkdawson commented 1 year ago

Testing this branch, I see a regression for #59, where svcli shows the same incorrect calculated checksum for the linked Firefox installer.

I don't understand this issue well enough to give useful input, but for what it's worth, CryptQueryObject fails to find an embedded signature in YourPhone.exe (error code CRYPT_E_NO_MATCH). This API does not check the Authenticode hash.

signtool reports the same:

SignTool Error: File not valid: YourPhone.exe
SignTool Error: No signature found.
woodruffw commented 1 year ago

Thanks for checking that -- I need to add a regression test for that as well. I won't merge until we get the correct hash for PEs that do contain signatures.

woodruffw commented 1 year ago

With the latest changes I now get the expected hashes for the testcase in #59:

./src/svcli/svcli ~/Downloads/Firefox\ Setup\ 101.0.exe        
This PE is verified!                                                                              

/Users/william/Downloads/Firefox Setup 101.0.exe has 1 certificate entries                        

Calculated checksums:                                                                             
   MD5: 0678B4315E48B222D3B54477E51C64F0                                                          
  SHA1: 69A54D16B3E176E16EF6AE5BBC6490F9F019A9B6                                                  
SHA256: DEF2F92C51E7DD222F480B9459C1E2E0A35C598453FC85EF74D9439D105AC853

I'm going to look into a smaller PE that I can include in the test suite to prevent regressions here.