Skip to content

gh-127667: fix more reference leaks in hashlib #127668

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
picnixz merged 18 commits into from
Mar 3, 2025
Merged

gh-127667: fix more reference leaks in hashlib #127668

Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
23eb1fe
fix error-branches in hashlib
picnixz Dec 4, 2024
49e6c96
small cosmetic changes in error messages
picnixz Dec 4, 2024
87a1e98
fix compilation
picnixz Dec 6, 2024
1b7c775
remove unused variable
picnixz Dec 6, 2024
2b20233
Merge branch 'main' into fix/hashlib/error-branches-127667
picnixz Dec 21, 2024
9e34c0e
only keep error-branches fixes
picnixz Jan 5, 2025
f06a52e
cosmetic alignment updates
picnixz Jan 5, 2025
4045980
avoid overwrite exception in `py_evp_fromname`
picnixz Jan 6, 2025
bf48fc2
remove un-necessary `PyErr_Occurred()` assertions
picnixz Jan 6, 2025
6f80c7c
suppress unused `_setException()` return value
picnixz Jan 6, 2025
fc3ebb0
collapse `_setException() + return NULL` into one statement
picnixz Jan 6, 2025
7455d3c
fix return type
picnixz Jan 6, 2025
add9e43
[empty commit] trigger RTD build
picnixz Jan 6, 2025
d72c096
[empty commit] trigger the CI
picnixz Jan 7, 2025
40c99d7
Merge branch 'main' into fix/hashlib/error-branches-127667
picnixz Jan 23, 2025
a4c5865
Merge remote-tracking branch 'upstream/main' into fix/hashlib/error-b…
picnixz Mar 2, 2025
bf83b61
post-merge
picnixz Mar 2, 2025
927ab4c
fix comment
picnixz Mar 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 78 additions & 58 deletions Modules/_hashopenssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,8 @@ py_digest_by_name(PyObject *module, const char *name, enum Py_hash_type py_ht)
}
}
if (digest == NULL) {
_setException(state->unsupported_digestmod_error, "unsupported hash type %s", name);
(void)_setException(state->unsupported_digestmod_error,
"unsupported hash type %s", name);
return NULL;
}
return digest;
Expand All @@ -445,7 +446,6 @@ py_digest_by_name(PyObject *module, const char *name, enum Py_hash_type py_ht)
*/
static PY_EVP_MD*
py_digest_by_digestmod(PyObject *module, PyObject *digestmod, enum Py_hash_type py_ht) {
PY_EVP_MD* evp;
PyObject *name_obj = NULL;
const char *name;

Expand All @@ -471,12 +471,7 @@ py_digest_by_digestmod(PyObject *module, PyObject *digestmod, enum Py_hash_type
return NULL;
}

evp = py_digest_by_name(module, name, py_ht);
if (evp == NULL) {
return NULL;
}

return evp;
return py_digest_by_name(module, name, py_ht);
}

static EVPobject *
Expand Down Expand Up @@ -509,7 +504,7 @@ EVP_hash(EVPobject *self, const void *vp, Py_ssize_t len)
else
process = Py_SAFE_DOWNCAST(len, Py_ssize_t, unsigned int);
if (!EVP_DigestUpdate(self->ctx, (const void*)cp, process)) {
_setException(PyExc_ValueError, NULL);
(void)_setException(PyExc_ValueError, NULL);
return -1;
}
len -= process;
Expand Down Expand Up @@ -586,17 +581,20 @@ EVP_digest_impl(EVPobject *self)
}

if (!locked_EVP_MD_CTX_copy(temp_ctx, self)) {
return _setException(PyExc_ValueError, NULL);
goto error;
}
digest_size = EVP_MD_CTX_size(temp_ctx);
if (!EVP_DigestFinal(temp_ctx, digest, NULL)) {
_setException(PyExc_ValueError, NULL);
return NULL;
goto error;
}

retval = PyBytes_FromStringAndSize((const char *)digest, digest_size);
EVP_MD_CTX_free(temp_ctx);
return retval;

error:
EVP_MD_CTX_free(temp_ctx);
return _setException(PyExc_ValueError, NULL);
}

/*[clinic input]
Expand All @@ -621,17 +619,20 @@ EVP_hexdigest_impl(EVPobject *self)

/* Get the raw (binary) digest value */
if (!locked_EVP_MD_CTX_copy(temp_ctx, self)) {
return _setException(PyExc_ValueError, NULL);
goto error;
}
digest_size = EVP_MD_CTX_size(temp_ctx);
if (!EVP_DigestFinal(temp_ctx, digest, NULL)) {
_setException(PyExc_ValueError, NULL);
return NULL;
goto error;
}

EVP_MD_CTX_free(temp_ctx);

return _Py_strhex((const char *)digest, (Py_ssize_t)digest_size);

error:
EVP_MD_CTX_free(temp_ctx);
return _setException(PyExc_ValueError, NULL);
}

/*[clinic input]
Expand Down Expand Up @@ -700,8 +701,11 @@ static PyObject *
EVP_get_name(PyObject *op, void *Py_UNUSED(closure))
{
EVPobject *self = EVPobject_CAST(op);
// NOTE(picnixz): NULL EVP context will be handled by gh-127667.
return py_digest_name(EVP_MD_CTX_md(self->ctx));
const EVP_MD *md = EVP_MD_CTX_md(self->ctx);
if (md == NULL) {
return _setException(PyExc_ValueError, NULL);
}
return py_digest_name(md);
}

static PyGetSetDef EVP_getseters[] = {
Expand Down Expand Up @@ -789,21 +793,22 @@ EVPXOF_digest_impl(EVPobject *self, Py_ssize_t length)
}

if (!locked_EVP_MD_CTX_copy(temp_ctx, self)) {
Py_DECREF(retval);
EVP_MD_CTX_free(temp_ctx);
return _setException(PyExc_ValueError, NULL);
goto error;
}
if (!EVP_DigestFinalXOF(temp_ctx,
(unsigned char*)PyBytes_AS_STRING(retval),
length)) {
Py_DECREF(retval);
EVP_MD_CTX_free(temp_ctx);
_setException(PyExc_ValueError, NULL);
return NULL;
length))
{
goto error;
}

EVP_MD_CTX_free(temp_ctx);
return retval;

error:
Py_DECREF(retval);
EVP_MD_CTX_free(temp_ctx);
return _setException(PyExc_ValueError, NULL);
}

/*[clinic input]
Expand Down Expand Up @@ -837,22 +842,22 @@ EVPXOF_hexdigest_impl(EVPobject *self, Py_ssize_t length)

/* Get the raw (binary) digest value */
if (!locked_EVP_MD_CTX_copy(temp_ctx, self)) {
PyMem_Free(digest);
EVP_MD_CTX_free(temp_ctx);
return _setException(PyExc_ValueError, NULL);
goto error;
}
if (!EVP_DigestFinalXOF(temp_ctx, digest, length)) {
PyMem_Free(digest);
EVP_MD_CTX_free(temp_ctx);
_setException(PyExc_ValueError, NULL);
return NULL;
goto error;
}

EVP_MD_CTX_free(temp_ctx);

retval = _Py_strhex((const char *)digest, length);
PyMem_Free(digest);
return retval;

error:
PyMem_Free(digest);
EVP_MD_CTX_free(temp_ctx);
return _setException(PyExc_ValueError, NULL);
}

static PyMethodDef EVPXOF_methods[] = {
Expand Down Expand Up @@ -950,7 +955,7 @@ py_evp_fromname(PyObject *module, const char *digestname, PyObject *data_obj,

int result = EVP_DigestInit_ex(self->ctx, digest, NULL);
if (!result) {
_setException(PyExc_ValueError, NULL);
(void)_setException(PyExc_ValueError, NULL);
Py_CLEAR(self);
goto exit;
}
Expand All @@ -971,7 +976,7 @@ py_evp_fromname(PyObject *module, const char *digestname, PyObject *data_obj,
}
}

exit:
exit:
if (data_obj != NULL) {
PyBuffer_Release(&view);
}
Expand Down Expand Up @@ -1416,14 +1421,14 @@ _hashlib_scrypt_impl(PyObject *module, Py_buffer *password, Py_buffer *salt,
r = PyLong_AsUnsignedLong(r_obj);
if (r == (unsigned long) -1 && PyErr_Occurred()) {
PyErr_SetString(PyExc_TypeError,
"r is required and must be an unsigned int");
"r is required and must be an unsigned int");
return NULL;
}

p = PyLong_AsUnsignedLong(p_obj);
if (p == (unsigned long) -1 && PyErr_Occurred()) {
PyErr_SetString(PyExc_TypeError,
"p is required and must be an unsigned int");
"p is required and must be an unsigned int");
return NULL;
}

Expand All @@ -1432,22 +1437,22 @@ _hashlib_scrypt_impl(PyObject *module, Py_buffer *password, Py_buffer *salt,
future. The maxmem constant is private to OpenSSL. */
PyErr_Format(PyExc_ValueError,
"maxmem must be positive and smaller than %d",
INT_MAX);
INT_MAX);
return NULL;
}

if (dklen < 1 || dklen > INT_MAX) {
PyErr_Format(PyExc_ValueError,
"dklen must be greater than 0 and smaller than %d",
INT_MAX);
"dklen must be greater than 0 and smaller than %d",
INT_MAX);
return NULL;
}

/* let OpenSSL validate the rest */
retval = EVP_PBE_scrypt(NULL, 0, NULL, 0, n, r, p, maxmem, NULL, 0);
if (!retval) {
_setException(PyExc_ValueError, "Invalid parameter combination for n, r, p, maxmem.");
return NULL;
return _setException(PyExc_ValueError,
"Invalid parameter combination for n, r, p, maxmem.");
}

key_obj = PyBytes_FromStringAndSize(NULL, dklen);
Expand All @@ -1467,8 +1472,7 @@ _hashlib_scrypt_impl(PyObject *module, Py_buffer *password, Py_buffer *salt,

if (!retval) {
Py_CLEAR(key_obj);
_setException(PyExc_ValueError, NULL);
return NULL;
return _setException(PyExc_ValueError, NULL);
}
return key_obj;
}
Expand Down Expand Up @@ -1524,8 +1528,7 @@ _hashlib_hmac_singleshot_impl(PyObject *module, Py_buffer *key,
PY_EVP_MD_free(evp);

if (result == NULL) {
_setException(PyExc_ValueError, NULL);
return NULL;
return _setException(PyExc_ValueError, NULL);
}
return PyBytes_FromStringAndSize((const char*)md, md_len);
}
Expand Down Expand Up @@ -1574,14 +1577,15 @@ _hashlib_hmac_new_impl(PyObject *module, Py_buffer *key, PyObject *msg_obj,

ctx = HMAC_CTX_new();
if (ctx == NULL) {
PY_EVP_MD_free(digest);
PyErr_NoMemory();
goto error;
}

r = HMAC_Init_ex(ctx, key->buf, (int)key->len, digest, NULL /* impl */);
PY_EVP_MD_free(digest);
if (r == 0) {
_setException(PyExc_ValueError, NULL);
(void)_setException(PyExc_ValueError, NULL);
goto error;
}

Expand Down Expand Up @@ -1619,12 +1623,20 @@ locked_HMAC_CTX_copy(HMAC_CTX *new_ctx_p, HMACobject *self)
return result;
}

/* returning 0 means that an error occurred and an exception is set */
static unsigned int
_hmac_digest_size(HMACobject *self)
{
// TODO(picnixz): NULL EVP context should also handled by gh-127667.
unsigned int digest_size = EVP_MD_size(HMAC_CTX_get_md(self->ctx));
const EVP_MD *md = HMAC_CTX_get_md(self->ctx);
if (md == NULL) {
(void)_setException(PyExc_ValueError, NULL);
return 0;
}
unsigned int digest_size = EVP_MD_size(md);
assert(digest_size <= EVP_MAX_MD_SIZE);
if (digest_size == 0) {
(void)_setException(PyExc_ValueError, NULL);
}
return digest_size;
}

Expand Down Expand Up @@ -1652,7 +1664,7 @@ _hmac_update(HMACobject *self, PyObject *obj)
PyBuffer_Release(&view);

if (r == 0) {
_setException(PyExc_ValueError, NULL);
(void)_setException(PyExc_ValueError, NULL);
return 0;
}
return 1;
Expand Down Expand Up @@ -1707,7 +1719,11 @@ static PyObject *
_hmac_repr(PyObject *op)
{
HMACobject *self = HMACobject_CAST(op);
PyObject *digest_name = py_digest_name(HMAC_CTX_get_md(self->ctx));
const EVP_MD *md = HMAC_CTX_get_md(self->ctx);
if (md == NULL) {
return _setException(PyExc_ValueError, NULL);
}
PyObject *digest_name = py_digest_name(md);
if (digest_name == NULL) {
return NULL;
}
Expand Down Expand Up @@ -1740,18 +1756,18 @@ _hmac_digest(HMACobject *self, unsigned char *buf, unsigned int len)
{
HMAC_CTX *temp_ctx = HMAC_CTX_new();
if (temp_ctx == NULL) {
PyErr_NoMemory();
(void)PyErr_NoMemory();
return 0;
}
if (!locked_HMAC_CTX_copy(temp_ctx, self)) {
HMAC_CTX_free(temp_ctx);
_setException(PyExc_ValueError, NULL);
(void)_setException(PyExc_ValueError, NULL);
return 0;
}
int r = HMAC_Final(temp_ctx, buf, &len);
HMAC_CTX_free(temp_ctx);
if (r == 0) {
_setException(PyExc_ValueError, NULL);
(void)_setException(PyExc_ValueError, NULL);
return 0;
}
return 1;
Expand All @@ -1769,7 +1785,7 @@ _hashlib_HMAC_digest_impl(HMACobject *self)
unsigned char digest[EVP_MAX_MD_SIZE];
unsigned int digest_size = _hmac_digest_size(self);
if (digest_size == 0) {
return _setException(PyExc_ValueError, NULL);
return NULL;
}
int r = _hmac_digest(self, digest, digest_size);
if (r == 0) {
Expand All @@ -1794,7 +1810,7 @@ _hashlib_HMAC_hexdigest_impl(HMACobject *self)
unsigned char digest[EVP_MAX_MD_SIZE];
unsigned int digest_size = _hmac_digest_size(self);
if (digest_size == 0) {
return _setException(PyExc_ValueError, NULL);
return NULL;
}
int r = _hmac_digest(self, digest, digest_size);
if (r == 0) {
Expand All @@ -1809,7 +1825,7 @@ _hashlib_hmac_get_digest_size(PyObject *op, void *Py_UNUSED(closure))
HMACobject *self = HMACobject_CAST(op);
unsigned int digest_size = _hmac_digest_size(self);
if (digest_size == 0) {
return _setException(PyExc_ValueError, NULL);
return NULL;
}
return PyLong_FromLong(digest_size);
}
Expand All @@ -1829,7 +1845,11 @@ static PyObject *
_hashlib_hmac_get_name(PyObject *op, void *Py_UNUSED(closure))
{
HMACobject *self = HMACobject_CAST(op);
PyObject *digest_name = py_digest_name(HMAC_CTX_get_md(self->ctx));
const EVP_MD *md = HMAC_CTX_get_md(self->ctx);
if (md == NULL) {
return _setException(PyExc_ValueError, NULL);
}
PyObject *digest_name = py_digest_name(md);
if (digest_name == NULL) {
return NULL;
}
Expand Down Expand Up @@ -1980,7 +2000,7 @@ _hashlib_get_fips_mode_impl(PyObject *module)
// But 0 is also a valid result value.
unsigned long errcode = ERR_peek_last_error();
if (errcode) {
_setException(PyExc_ValueError, NULL);
(void)_setException(PyExc_ValueError, NULL);
return -1;
}
}
Expand Down
Loading