mtrojnar / osslsigncode

OpenSSL based Authenticode signing for PE/MSI/Java CAB files
Other
806 stars 131 forks source link

Use-after-free when talking to PKCS11 #410

Closed sgtatham closed 3 weeks ago

sgtatham commented 1 month ago

I'm setting up osslsigncode to work with a SafeNet hardware token, via the PKCS11 driver library libeTPkcs11.so provided by SafeNet.

Running a command of this form (details specific to my token omitted):

osslsigncode sign \
     -pkcs11module /usr/lib/libeTPkcs11.so \
     -key ${keyid?} -pkcs11cert ${certid?} \
     -readpass ${passwordfile?} \
     -in test.exe -out signed.exe

I found osslsigncode segfaulted. I believe this occurs because of this code at the end of read_token() in osslsigncode.c:

    /* Free the functional reference from ENGINE_init */
    ENGINE_finish(engine);
    if (!options->pkey) {
        fprintf(stderr, "Failed to load private key %s\n", options->keyfile);
        return 0; /* FAILED */
    }
    return 1; /* OK */

in which the call to ENGINE_finish frees a data structure that is needed by the later signing operation from within pkcs7_sign_content.

Details of investigation: a gdb backtrace from the segfault pointed to somewhere deep in libengine-pkcs11-openssl:

(gdb) ba
#0  0x00007ffff7eb81fe in ?? () from /usr/lib/x86_64-linux-gnu/engines-3/pkcs11.so
#1  0x00007ffff7eb8962 in ?? () from /usr/lib/x86_64-linux-gnu/engines-3/pkcs11.so
#2  0x00007ffff7ebe567 in ?? () from /usr/lib/x86_64-linux-gnu/engines-3/pkcs11.so
#3  0x00007ffff7ebe8b0 in ?? () from /usr/lib/x86_64-linux-gnu/engines-3/pkcs11.so
#4  0x00007ffff7ebc731 in ?? () from /usr/lib/x86_64-linux-gnu/engines-3/pkcs11.so
#5  0x00007ffff7ebc7bb in ?? () from /usr/lib/x86_64-linux-gnu/engines-3/pkcs11.so
#6  0x00007ffff7b1eed6 in RSA_sign () from /lib/x86_64-linux-gnu/libcrypto.so.3
#7  0x00007ffff7b1d5a2 in ?? () from /lib/x86_64-linux-gnu/libcrypto.so.3
#8  0x00007ffff7a06817 in EVP_DigestSignFinal () from /lib/x86_64-linux-gnu/libcrypto.so.3
#9  0x00007ffff7afdcbc in PKCS7_SIGNER_INFO_sign () from /lib/x86_64-linux-gnu/libcrypto.so.3
#10 0x00007ffff7afdf9d in PKCS7_dataFinal () from /lib/x86_64-linux-gnu/libcrypto.so.3
#11 0x000055555556f7bf in pkcs7_sign_content ()
#12 0x000055555556f26a in sign_spc_indirect_data_content ()
#13 0x0000555555579b9d in pe_pkcs7_signature_new ()
#14 0x000055555556e542 in main ()

To get more detail, I recompiled pkcs11.so from the Ubuntu package source with debugging enabled, and reproduced the segfault with the rebuilt library. The immediate cause of the segfault was that pkcs11_getattr_var was being called with a ctx pointer that looked like 64 bits of binary nonsense and not a sensible address. Tracing back up, that pointer had been read out of a PKCS11_OBJECT_private *key by accessing key->slot->ctx.

By breakpointing pkcs11_slot_new I observed that slot structure being created with a sensible ctx value; by watchpointing that to see when its value changed, I found that it was becoming corrupted by a free() arising from the call to ENGINE_finish() in osslsigncode's read_token(). But apparently a pointer to the same slot structure was retained somewhere, and later read by the code called from pkcs7_sign_content.

My workaround is to remove the call to ENGINE_finish() completely, on the path leading to a success return from read_token(). I'm sure that's not a high-quality patch (probably the memory ought to be freed somewhere else instead), but it's prevented the segfault for me.

sgtatham commented 1 month ago

Possibly related to #388? But the segfault reported there is in a different location.

Version details: I had built osslsigncode from the current master, which is commit 185cc0748dd5aeb867a9b5cddcd5ecdef06a04de. I'm on Ubuntu 24.04, with relevant package versions

mtrojnar commented 1 month ago

Can you reproduce this issue with the latest master branch of libp11?

sgtatham commented 1 month ago

Yes, I just tried with commit https://github.com/OpenSC/libp11/commit/1d23e0cedcd3bd1b579e0b108445292ea29ac8c2. Same behaviour: a segfault in RSA_sign, which goes away if I comment out that call to ENGINE_finish in osslsigncode's read_token.

mtrojnar commented 1 month ago

@olszomal Could you take a look at it?

mtrojnar commented 3 weeks ago

@sgtatham Could you try again using pkcs11.so built from the latest master branch of https://github.com/OpenSC/libp11?

sgtatham commented 3 weeks ago

Looks good! Having first replicated the previous crash with https://github.com/OpenSC/libp11/commit/1d23e0cedcd3bd1b579e0b108445292ea29ac8c2, I updated to https://github.com/OpenSC/libp11/commit/ba4ae57f05ed708c4e2b88561893aa9addb9ee47 changing nothing else. This time I get no segfault, and see the output

Engine "pkcs11" set.
Workaround for OpenSSL 3.0.13 30 Jan 2024 enabled
Succeeded

and afterwards I have a correctly signed output file.

mtrojnar commented 3 weeks ago

Thank you for testing.