]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-127667: fix memory leaks in `hashlib` (#127668)
authorBénédikt Tran <10796600+picnixz@users.noreply.github.com>
Mon, 3 Mar 2025 08:20:33 +0000 (09:20 +0100)
committerGitHub <noreply@github.com>
Mon, 3 Mar 2025 08:20:33 +0000 (09:20 +0100)
- Correctly handle `NULL` values returned by `EVP_MD_CTX_md`.
- Correctly free resources in error branches.
- Consistently suppress `_setException()` return value when needed.
- Collapse `_setException() + return NULL` into a single statement.

Modules/_hashopenssl.c

index f66b182b1deb6aa334a4ea6ba16bf711d930f8ba..4d6e4f192f5ad9b2ac3446109ca633bb02e4fbf6 100644 (file)
@@ -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;
@@ -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;
 
@@ -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 *
@@ -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;
@@ -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]
@@ -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]
@@ -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[] = {
@@ -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]
@@ -837,15 +842,10 @@ 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);
@@ -853,6 +853,11 @@ EVPXOF_hexdigest_impl(EVPobject *self, Py_ssize_t length)
     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[] = {
@@ -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;
     }
@@ -971,7 +976,7 @@ py_evp_fromname(PyObject *module, const char *digestname, PyObject *data_obj,
         }
     }
 
-  exit:
+exit:
     if (data_obj != NULL) {
         PyBuffer_Release(&view);
     }
@@ -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;
     }
 
@@ -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);
@@ -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;
 }
@@ -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);
 }
@@ -1574,6 +1577,7 @@ _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;
     }
@@ -1581,7 +1585,7 @@ _hashlib_hmac_new_impl(PyObject *module, Py_buffer *key, PyObject *msg_obj,
     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;
     }
 
@@ -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;
 }
 
@@ -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;
@@ -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;
     }
@@ -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;
@@ -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) {
@@ -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) {
@@ -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);
 }
@@ -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;
     }
@@ -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;
         }
     }