Am 12.02.2017 um 18:54 schrieb Kevin J. McCarthy:
> On Sun, Feb 12, 2017 at 06:31:59PM +0100, Matthias Andree wrote:
>> Am 12.02.2017 um 17:35 schrieb Kevin J. McCarthy:
>>> - while ((cert = PEM_read_X509 (fp, NULL, NULL, NULL)) != NULL)
>>> + while ((cert = PEM_read_X509 (fp, &cert, NULL, NULL)) != NULL)
>>> {
>>> if ((X509_cmp_current_time (X509_get_notBefore (cert)) >= 0) ||
>>> (X509_cmp_current_time (X509_get_notAfter (cert)) <= 0))
>>> {
>>> dprint (2, (debugfile, "ssl_load_certificates: filtering expired
>>> cert: %s\n",
>>> X509_NAME_oneline (X509_get_subject_name (cert), buf, sizeof
>>> (buf))));
>>> - X509_free (cert);
>>> }
>>> else
>>> + {
>>> X509_STORE_add_cert (store, cert);
>>> + }
>>> }
>>> + X509_free (cert);
>> This won't work, you'll need to rewrite the while loop:
>>
>> while (NULL != PEM_read_X509 (fp, &cert, NULL, NULL)) { ... }
>>
>> because otherwise you'll clobber the old value of 'cert' the moment
>> PEM_read_X509 returns NULL,
>> and then you've trashed the only pointer you hold to cert, leaking
>> memory again.
>>
>> The attached patch, incremental to my earlier one, achieves this.
> I think it depends whether PEM_read_X509() frees the cert for us in the
> EOF case. check_certificate_by_digest() has a memory leak if that's not
> the case, but the new version would double-free if it did.
It's not documented to free anything, and it doesn't zero out the
pointer, and with OpenSSL 1.0.2, valgrind does not complain about
freeing free memory.
Unfortunately OpenSSL (ab)uses #define preprocessor macros with
concatenation (##) for the poor man's generic programming, and that
makes it hard to trace, you need to look for PEM_ASN1_read in
crypto/pem/pem_lib.c, which calls upon PEM_ASN1_read_bio from
.../pem_oth.c, and then PEM_bytes_read_bio again from pem_lib.c, and
there I'm losing track while chasing down the d2i implementation.
