Public-key callback context setters leave a dangling wolfSSL pointer when replacement fails

[K-ANOY]

I found a use-after-free / double-free hazard in the seven public-key callback context setters in WolfSSLSession. Each one frees the old internCtx before allocating the replacement, but never clears the pointer stored inside WOLFSSL, so if the replacement allocation or its global reference fails, the session is left holding a freed pointer.

File: native/com_wolfssl_WolfSSLSession.c

Affected functions (same pattern): setEccSignCtx, setEccVerifyCtx,
setEccSharedSecretCtx, setRsaSignCtx, setRsaVerifyCtx, setRsaEncCtx,
setRsaDecCtx.

Relevant code (setEccVerifyCtx, the others are identical):

eccVerifyCtx = (internCtx*)wolfSSL_GetEccVerifyCtx(ssl);   // old ctx, still stored in ssl

if (eccVerifyCtx != NULL) {
    myCtx = (internCtx*)eccVerifyCtx;
    if (myCtx != NULL) {
        if (myCtx->active == 1) {
            (*jenv)->DeleteGlobalRef(jenv, myCtx->obj);
        }
        XFREE(myCtx, NULL, DYNAMIC_TYPE_TMP_BUFFER);       // old ctx freed; ssl slot NOT cleared
    }
}

myCtx = XMALLOC(sizeof(internCtx), NULL, DYNAMIC_TYPE_TMP_BUFFER);
if (!myCtx) {
    throwWolfSSLException(jenv, "Unable to allocate memory for ECC verify context\n");
    return;                                                // ssl still holds the freed old pointer
}

myCtx->active = 1;
myCtx->obj = (*jenv)->NewGlobalRef(jenv, jcl);
if (myCtx->obj == NULL) {
    throwWolfSSLException(jenv, "Unable to store WolfSSLSession object as global reference");
    return;                                                // freed old pointer still stored; new myCtx leaked
}

wolfSSL_SetEccVerifyCtx(ssl, myCtx);                       // only now is the slot overwritten

The old pointer returned by wolfSSL_GetEccVerifyCtx() stays inside ssl until the
final wolfSSL_SetEccVerifyCtx(). Both early returns in between leave ssl holding
a freed internCtx. That dangling pointer is later consumed by freeSSL(), which
reads pkCtx->obj and frees it again:

pkCtx = (internCtx*)wolfSSL_GetEccVerifyCtx(ssl);
if (pkCtx != NULL) {
    if (pkCtx->obj != NULL) {
        (*jenv)->DeleteGlobalRef(jenv, pkCtx->obj);   // use-after-free read
    }
    XFREE(pkCtx, NULL, DYNAMIC_TYPE_TMP_BUFFER);       // double free
}

so closing the session (a natural reaction to the exception) is itself one of the
paths that dereferences and double-frees the dangling pointer; a public-key callback
firing before cleanup does the same.

There is also a secondary leak on the NewGlobalRef() failure branch: six of the
seven setters return without freeing the newly allocated myCtx. Only
setEccSignCtx frees it (XFREE(myCtx, ...) before its return); setEccVerifyCtx,
setEccSharedSecretCtx, setRsaSignCtx, setRsaVerifyCtx, setRsaEncCtx, and
setRsaDecCtx do not. (All seven still leave the freed old pointer installed.)

These setters back the public WolfSSLSession.setEcc*/setRsa*Ctx(Object) methods, so
the branch is reachable from Java; it requires XMALLOC() or NewGlobalRef() to
fail, but once taken the corruption is deterministic. API_BEGIN()/API_END() does not
help — the pointers are raw and a plain return does no cleanup.

Suggested fix: build the complete replacement first, install it, and only then
destroy the old context, so a failure never leaves a freed pointer stored in ssl:

internCtx* oldCtx = (internCtx*)wolfSSL_GetEccVerifyCtx(ssl);

internCtx* newCtx = XMALLOC(sizeof(internCtx), NULL, DYNAMIC_TYPE_TMP_BUFFER);
if (newCtx == NULL) {
    throwWolfSSLException(jenv, "Unable to allocate memory for ECC verify context");
    return;                                  /* old ctx still installed and valid */
}
newCtx->active = 1;
newCtx->obj = (*jenv)->NewGlobalRef(jenv, jcl);
if (newCtx->obj == NULL) {
    XFREE(newCtx, NULL, DYNAMIC_TYPE_TMP_BUFFER);   /* free only the uninstalled replacement */
    return;
}

wolfSSL_SetEccVerifyCtx(ssl, newCtx);        /* atomically swap in the new pointer */

if (oldCtx != NULL) {
    if (oldCtx->active == 1 && oldCtx->obj != NULL) {
        (*jenv)->DeleteGlobalRef(jenv, oldCtx->obj);
    }
    XFREE(oldCtx, NULL, DYNAMIC_TYPE_TMP_BUFFER);
}

With this ordering freeSSL() always observes either the valid old context or the
valid new one, and the NewGlobalRef() failure path frees only the uninstalled
replacement. The same reordering should be applied to all seven setters.

[stenslae]

Thanks for the report, @K-ANOY!

I added a fix for the issue on PR #377.

Best Regards,
Emma Stensland