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

Reply via email to