CVSROOT: /cvs Module name: src Changes by: t...@cvs.openbsd.org 2025/07/27 01:06:41
Modified files: lib/libcrypto/pkcs7: pk7_doit.c Log message: Fix incorrect ownership handling in add_attribute() This little gem has a number of issues. On failure, the caller can't know whether ownership of value was taken or not, so to avoid a double free, the only option is to leak value on failure. As X509_ATTRIBUTE_create() takes ownership on success, this call must be the last one that can fail. This way ownership is only taken on success. Next, if X509_ATTRIBUTE_create() fails in the case that the input stack already contains an attribute of type nid, that attr is freed and the caller freeing the stack with pop_free() will double free. So, rework this in a few ways. Make this transactional, so we don't fail with a modified *in_sk, so work with a local sk as usual. Then walk the stack and see if we have an attribute with the appropriate nid already. If not, make sure there's room to place the new attribute. Create the new attribute, free the old attribute if necessary and replace it with the new one. Finally assign the local sk to *in_sk and return success. On error unwind all we did. The behavior now matches OpenSSL 3's new behavior, except that we don't leave an empty stack around on error. ok kenjiro