segfault with squid2.5 + ssl accel patch (and fix)

I think I have found the source of a pesky segfault in the ssl accel
patch (for squid 2.5 stable 12). Essentially, there are two functions
calling CRYPTO_free() when they should not be. Here is the explanation
with respect to sslGetUserAttribute(), but sslGetCAAttribute() follows
the same logic and so has the same problem.

Here is the function:

const char *
sslGetUserAttribute(SSL * ssl, const char *attribute_name)
{
    X509 *cert;
    X509_NAME *name;
    const char *ret;

    if (!ssl)
        return NULL;

    cert = SSL_get_peer_certificate(ssl);
    if (!cert)
        return NULL;

    name = X509_get_subject_name(cert);

    ret = ssl_get_attribute(name, attribute_name);

    X509_free(cert);
    CRYPTO_free(name);

    return ret;
}

SSL_get_peer_certificate() returns a pointer to an X509 struct
previously allocated somewhere on the heap. This is an internally
refcounted structure, and the call to X509_free() at the bottom only
actually frees the memory for the X509 struct if the ref count drops to
0. In this case, it is probably only decrementing the refcount and not
actually freeing any memory.

Now, X509_get_subject_name() simply returns a pointer to some data
inside that X509 cert struct. The problem is that CRYPTO_free() doesn't
pay attention to ref counts or anything; it is just a wrapper for
free(), so you end up basically calling free() on some memory that
belongs to a persistent refcounted struct that would be cleaned up by
X509_free() once the refcount drops to 0 anyways. The problem manifests
itself quite clearly if you try to call sslGetUserAttribute with the
same SSL * more than once (I.e. try to setup two user_cert acls in
squid.conf).

The fix is simply to remove the two calls to CRYPTO_free() in
ssl_support.c.

Hope this makes sense.

-Andrew
Received on Tue Dec 20 2005 - 19:25:52 MST