fix: verify xku_flags with XKU_CODE_SIGN or XKU_TIMESTAMP

[zeze-zeze]

#102

[CLAassistant]

All committers have signed the CLA.

[dguido]

@woodruffw Thank you for catching this critical security issue! You're absolutely right - simply allowing TSA certificates (as this PR originally attempted) would create a dangerous vulnerability.

I've pushed a new commit that implements the proper fix by filtering out TSA certificates entirely before passing them to PKCS7_verify. Here's the approach:

The Security Issue

As you correctly identified, if TSA certificates are included in the PKCS7_verify call, an attacker could bypass Authenticode verification by having a valid signature against a TSA cert instead of against a proper code-signing cert.

The Fix

The updated implementation (commit b613d9f):

1. Reverts the XKU check for signers back to requiring only XKU_CODE_SIGN (line 245)
2. Creates a filtered certificate stack that excludes TSA-only certificates
3. Filtering logic:
   • Skip certificates where xku_flags == XKU_TIMESTAMP (TSA-only certs)
   • Require XKU_CODE_SIGN flag for all other certificates
   • Build a new stack with only valid code-signing certificates
4. Passes the filtered stack to PKCS7_verify instead of the original certificate list

This ensures TSA certificates cannot be used as signers while still allowing proper code-signing certificates through.

Key Changes in src/uthenticode.cpp:

• Lines 250-280: New filtering logic that builds filtered_certs stack
• Line 265: Skip TSA-only certificates (xku_flags == XKU_TIMESTAMP)
• Line 321: Pass filtered_certs to PKCS7_verify instead of original certs
• Proper memory management with sk_X509_free(filtered_certs) on all paths

The test file has also been updated to document this security fix rather than the previous (vulnerable) approach.

This should properly address the security concern while still fixing the original issue in #102. Let me know if this approach looks correct!