Release i2s_ASN1_OCTET_STRING output with OPENSSL_free

[achamayou]

Summary

subject_key_id() and authority_key_id() released the string returned by i2s_ASN1_OCTET_STRING with the CRT free():

const std::unique_ptr<char, decltype(&free)> c(i2s_ASN1_OCTET_STRING(nullptr, key_id), free);
return {c.get()};

i2s_ASN1_OCTET_STRING allocates its result with OPENSSL_malloc. When OpenSSL is built against a different allocator than the CRT (notably on Windows, or with a custom CRYPTO_set_mem_functions), releasing it with free() corrupts the heap.

There is also a latent UB: if i2s_ASN1_OCTET_STRING returns nullptr, return {c.get()} constructs std::string(nullptr).

Fix

• Release the buffer with OPENSSL_free via a unique_ptr deleter.
• CHECKNULL the result so a nullptr becomes a thrown exception rather than UB.

Both methods are currently unused, so this is a portability/correctness hardening fix that prevents a heap-corruption bug the moment a caller is added.

Testing

• cmake --build build — clean.
• ctest — all tests pass.
• clang-tidy ../didx509cpp.h — exit 0, no findings (mirrors CI).

Found during a security review of the codebase.

[Copilot]

Pull request overview

This PR hardens the didx509::X509 key identifier helpers against allocator-mismatch bugs by ensuring buffers returned from OpenSSL are freed with OpenSSL’s allocator and by converting potential nullptr returns into exceptions.

Changes:

• Replace CRT free() with OPENSSL_free for buffers returned by i2s_ASN1_OCTET_STRING.
• Add CHECKNULL after i2s_ASN1_OCTET_STRING to avoid std::string(nullptr) UB and provide a defined failure mode.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.