Skip to content

Commit 0978465

Browse files
picnixzpicnixz
authored
pythongh-127667: fix memory leaks in hashlib (python#127668)
- 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.
1 parent 7e3b788 commit 0978465

File tree

1 file changed

+78
-58
lines changed

1 file changed

+78
-58
lines changed

‎Modules/_hashopenssl.c‎

Lines changed: 78 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,8 @@ py_digest_by_name(PyObject *module, const char *name, enum Py_hash_type py_ht)
430430
}
431431
}
432432
if (digest == NULL) {
433-
_setException(state->unsupported_digestmod_error, "unsupported hash type %s", name);
433+
(void)_setException(state->unsupported_digestmod_error,
434+
"unsupported hash type %s", name);
434435
return NULL;
435436
}
436437
return digest;
@@ -445,7 +446,6 @@ py_digest_by_name(PyObject *module, const char *name, enum Py_hash_type py_ht)
445446
*/
446447
static PY_EVP_MD*
447448
py_digest_by_digestmod(PyObject *module, PyObject *digestmod, enum Py_hash_type py_ht) {
448-
PY_EVP_MD* evp;
449449
PyObject *name_obj = NULL;
450450
const char *name;
451451

@@ -471,12 +471,7 @@ py_digest_by_digestmod(PyObject *module, PyObject *digestmod, enum Py_hash_type
471471
return NULL;
472472
}
473473

474-
evp = py_digest_by_name(module, name, py_ht);
475-
if (evp == NULL) {
476-
return NULL;
477-
}
478-
479-
return evp;
474+
return py_digest_by_name(module, name, py_ht);
480475
}
481476

482477
static EVPobject *
@@ -509,7 +504,7 @@ EVP_hash(EVPobject *self, const void *vp, Py_ssize_t len)
509504
else
510505
process = Py_SAFE_DOWNCAST(len, Py_ssize_t, unsigned int);
511506
if (!EVP_DigestUpdate(self->ctx, (const void*)cp, process)) {
512-
_setException(PyExc_ValueError, NULL);
507+
(void)_setException(PyExc_ValueError, NULL);
513508
return -1;
514509
}
515510
len -= process;
@@ -586,17 +581,20 @@ EVP_digest_impl(EVPobject *self)
586581
}
587582

588583
if (!locked_EVP_MD_CTX_copy(temp_ctx, self)) {
589-
return _setException(PyExc_ValueError, NULL);
584+
goto error;
590585
}
591586
digest_size = EVP_MD_CTX_size(temp_ctx);
592587
if (!EVP_DigestFinal(temp_ctx, digest, NULL)) {
593-
_setException(PyExc_ValueError, NULL);
594-
return NULL;
588+
goto error;
595589
}
596590

597591
retval = PyBytes_FromStringAndSize((const char *)digest, digest_size);
598592
EVP_MD_CTX_free(temp_ctx);
599593
return retval;
594+
595+
error:
596+
EVP_MD_CTX_free(temp_ctx);
597+
return _setException(PyExc_ValueError, NULL);
600598
}
601599

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

622620
/* Get the raw (binary) digest value */
623621
if (!locked_EVP_MD_CTX_copy(temp_ctx, self)) {
624-
return _setException(PyExc_ValueError, NULL);
622+
goto error;
625623
}
626624
digest_size = EVP_MD_CTX_size(temp_ctx);
627625
if (!EVP_DigestFinal(temp_ctx, digest, NULL)) {
628-
_setException(PyExc_ValueError, NULL);
629-
return NULL;
626+
goto error;
630627
}
631628

632629
EVP_MD_CTX_free(temp_ctx);
633630

634631
return _Py_strhex((const char *)digest, (Py_ssize_t)digest_size);
632+
633+
error:
634+
EVP_MD_CTX_free(temp_ctx);
635+
return _setException(PyExc_ValueError, NULL);
635636
}
636637

637638
/*[clinic input]
@@ -700,8 +701,11 @@ static PyObject *
700701
EVP_get_name(PyObject *op, void *Py_UNUSED(closure))
701702
{
702703
EVPobject *self = EVPobject_CAST(op);
703-
// NOTE(picnixz): NULL EVP context will be handled by gh-127667.
704-
return py_digest_name(EVP_MD_CTX_md(self->ctx));
704+
const EVP_MD *md = EVP_MD_CTX_md(self->ctx);
705+
if (md == NULL) {
706+
return _setException(PyExc_ValueError, NULL);
707+
}
708+
return py_digest_name(md);
705709
}
706710

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

791795
if (!locked_EVP_MD_CTX_copy(temp_ctx, self)) {
792-
Py_DECREF(retval);
793-
EVP_MD_CTX_free(temp_ctx);
794-
return _setException(PyExc_ValueError, NULL);
796+
goto error;
795797
}
796798
if (!EVP_DigestFinalXOF(temp_ctx,
797799
(unsigned char*)PyBytes_AS_STRING(retval),
798-
length)) {
799-
Py_DECREF(retval);
800-
EVP_MD_CTX_free(temp_ctx);
801-
_setException(PyExc_ValueError, NULL);
802-
return NULL;
800+
length))
801+
{
802+
goto error;
803803
}
804804

805805
EVP_MD_CTX_free(temp_ctx);
806806
return retval;
807+
808+
error:
809+
Py_DECREF(retval);
810+
EVP_MD_CTX_free(temp_ctx);
811+
return _setException(PyExc_ValueError, NULL);
807812
}
808813

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

838843
/* Get the raw (binary) digest value */
839844
if (!locked_EVP_MD_CTX_copy(temp_ctx, self)) {
840-
PyMem_Free(digest);
841-
EVP_MD_CTX_free(temp_ctx);
842-
return _setException(PyExc_ValueError, NULL);
845+
goto error;
843846
}
844847
if (!EVP_DigestFinalXOF(temp_ctx, digest, length)) {
845-
PyMem_Free(digest);
846-
EVP_MD_CTX_free(temp_ctx);
847-
_setException(PyExc_ValueError, NULL);
848-
return NULL;
848+
goto error;
849849
}
850850

851851
EVP_MD_CTX_free(temp_ctx);
852852

853853
retval = _Py_strhex((const char *)digest, length);
854854
PyMem_Free(digest);
855855
return retval;
856+
857+
error:
858+
PyMem_Free(digest);
859+
EVP_MD_CTX_free(temp_ctx);
860+
return _setException(PyExc_ValueError, NULL);
856861
}
857862

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

951956
int result = EVP_DigestInit_ex(self->ctx, digest, NULL);
952957
if (!result) {
953-
_setException(PyExc_ValueError, NULL);
958+
(void)_setException(PyExc_ValueError, NULL);
954959
Py_CLEAR(self);
955960
goto exit;
956961
}
@@ -971,7 +976,7 @@ py_evp_fromname(PyObject *module, const char *digestname, PyObject *data_obj,
971976
}
972977
}
973978

974-
exit:
979+
exit:
975980
if (data_obj != NULL) {
976981
PyBuffer_Release(&view);
977982
}
@@ -1416,14 +1421,14 @@ _hashlib_scrypt_impl(PyObject *module, Py_buffer *password, Py_buffer *salt,
14161421
r = PyLong_AsUnsignedLong(r_obj);
14171422
if (r == (unsigned long) -1 && PyErr_Occurred()) {
14181423
PyErr_SetString(PyExc_TypeError,
1419-
"r is required and must be an unsigned int");
1424+
"r is required and must be an unsigned int");
14201425
return NULL;
14211426
}
14221427

14231428
p = PyLong_AsUnsignedLong(p_obj);
14241429
if (p == (unsigned long) -1 && PyErr_Occurred()) {
14251430
PyErr_SetString(PyExc_TypeError,
1426-
"p is required and must be an unsigned int");
1431+
"p is required and must be an unsigned int");
14271432
return NULL;
14281433
}
14291434

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

14391444
if (dklen < 1 || dklen > INT_MAX) {
14401445
PyErr_Format(PyExc_ValueError,
1441-
"dklen must be greater than 0 and smaller than %d",
1442-
INT_MAX);
1446+
"dklen must be greater than 0 and smaller than %d",
1447+
INT_MAX);
14431448
return NULL;
14441449
}
14451450

14461451
/* let OpenSSL validate the rest */
14471452
retval = EVP_PBE_scrypt(NULL, 0, NULL, 0, n, r, p, maxmem, NULL, 0);
14481453
if (!retval) {
1449-
_setException(PyExc_ValueError, "Invalid parameter combination for n, r, p, maxmem.");
1450-
return NULL;
1454+
return _setException(PyExc_ValueError,
1455+
"Invalid parameter combination for n, r, p, maxmem.");
14511456
}
14521457

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

14681473
if (!retval) {
14691474
Py_CLEAR(key_obj);
1470-
_setException(PyExc_ValueError, NULL);
1471-
return NULL;
1475+
return _setException(PyExc_ValueError, NULL);
14721476
}
14731477
return key_obj;
14741478
}
@@ -1524,8 +1528,7 @@ _hashlib_hmac_singleshot_impl(PyObject *module, Py_buffer *key,
15241528
PY_EVP_MD_free(evp);
15251529

15261530
if (result == NULL) {
1527-
_setException(PyExc_ValueError, NULL);
1528-
return NULL;
1531+
return _setException(PyExc_ValueError, NULL);
15291532
}
15301533
return PyBytes_FromStringAndSize((const char*)md, md_len);
15311534
}
@@ -1574,14 +1577,15 @@ _hashlib_hmac_new_impl(PyObject *module, Py_buffer *key, PyObject *msg_obj,
15741577

15751578
ctx = HMAC_CTX_new();
15761579
if (ctx == NULL) {
1580+
PY_EVP_MD_free(digest);
15771581
PyErr_NoMemory();
15781582
goto error;
15791583
}
15801584

15811585
r = HMAC_Init_ex(ctx, key->buf, (int)key->len, digest, NULL /* impl */);
15821586
PY_EVP_MD_free(digest);
15831587
if (r == 0) {
1584-
_setException(PyExc_ValueError, NULL);
1588+
(void)_setException(PyExc_ValueError, NULL);
15851589
goto error;
15861590
}
15871591

@@ -1619,12 +1623,20 @@ locked_HMAC_CTX_copy(HMAC_CTX *new_ctx_p, HMACobject *self)
16191623
return result;
16201624
}
16211625

1626+
/* returning 0 means that an error occurred and an exception is set */
16221627
static unsigned int
16231628
_hmac_digest_size(HMACobject *self)
16241629
{
1625-
// TODO(picnixz): NULL EVP context should also handled by gh-127667.
1626-
unsigned int digest_size = EVP_MD_size(HMAC_CTX_get_md(self->ctx));
1630+
const EVP_MD *md = HMAC_CTX_get_md(self->ctx);
1631+
if (md == NULL) {
1632+
(void)_setException(PyExc_ValueError, NULL);
1633+
return 0;
1634+
}
1635+
unsigned int digest_size = EVP_MD_size(md);
16271636
assert(digest_size <= EVP_MAX_MD_SIZE);
1637+
if (digest_size == 0) {
1638+
(void)_setException(PyExc_ValueError, NULL);
1639+
}
16281640
return digest_size;
16291641
}
16301642

@@ -1652,7 +1664,7 @@ _hmac_update(HMACobject *self, PyObject *obj)
16521664
PyBuffer_Release(&view);
16531665

16541666
if (r == 0) {
1655-
_setException(PyExc_ValueError, NULL);
1667+
(void)_setException(PyExc_ValueError, NULL);
16561668
return 0;
16571669
}
16581670
return 1;
@@ -1707,7 +1719,11 @@ static PyObject *
17071719
_hmac_repr(PyObject *op)
17081720
{
17091721
HMACobject *self = HMACobject_CAST(op);
1710-
PyObject *digest_name = py_digest_name(HMAC_CTX_get_md(self->ctx));
1722+
const EVP_MD *md = HMAC_CTX_get_md(self->ctx);
1723+
if (md == NULL) {
1724+
return _setException(PyExc_ValueError, NULL);
1725+
}
1726+
PyObject *digest_name = py_digest_name(md);
17111727
if (digest_name == NULL) {
17121728
return NULL;
17131729
}
@@ -1740,18 +1756,18 @@ _hmac_digest(HMACobject *self, unsigned char *buf, unsigned int len)
17401756
{
17411757
HMAC_CTX *temp_ctx = HMAC_CTX_new();
17421758
if (temp_ctx == NULL) {
1743-
PyErr_NoMemory();
1759+
(void)PyErr_NoMemory();
17441760
return 0;
17451761
}
17461762
if (!locked_HMAC_CTX_copy(temp_ctx, self)) {
17471763
HMAC_CTX_free(temp_ctx);
1748-
_setException(PyExc_ValueError, NULL);
1764+
(void)_setException(PyExc_ValueError, NULL);
17491765
return 0;
17501766
}
17511767
int r = HMAC_Final(temp_ctx, buf, &len);
17521768
HMAC_CTX_free(temp_ctx);
17531769
if (r == 0) {
1754-
_setException(PyExc_ValueError, NULL);
1770+
(void)_setException(PyExc_ValueError, NULL);
17551771
return 0;
17561772
}
17571773
return 1;
@@ -1769,7 +1785,7 @@ _hashlib_HMAC_digest_impl(HMACobject *self)
17691785
unsigned char digest[EVP_MAX_MD_SIZE];
17701786
unsigned int digest_size = _hmac_digest_size(self);
17711787
if (digest_size == 0) {
1772-
return _setException(PyExc_ValueError, NULL);
1788+
return NULL;
17731789
}
17741790
int r = _hmac_digest(self, digest, digest_size);
17751791
if (r == 0) {
@@ -1794,7 +1810,7 @@ _hashlib_HMAC_hexdigest_impl(HMACobject *self)
17941810
unsigned char digest[EVP_MAX_MD_SIZE];
17951811
unsigned int digest_size = _hmac_digest_size(self);
17961812
if (digest_size == 0) {
1797-
return _setException(PyExc_ValueError, NULL);
1813+
return NULL;
17981814
}
17991815
int r = _hmac_digest(self, digest, digest_size);
18001816
if (r == 0) {
@@ -1809,7 +1825,7 @@ _hashlib_hmac_get_digest_size(PyObject *op, void *Py_UNUSED(closure))
18091825
HMACobject *self = HMACobject_CAST(op);
18101826
unsigned int digest_size = _hmac_digest_size(self);
18111827
if (digest_size == 0) {
1812-
return _setException(PyExc_ValueError, NULL);
1828+
return NULL;
18131829
}
18141830
return PyLong_FromLong(digest_size);
18151831
}
@@ -1829,7 +1845,11 @@ static PyObject *
18291845
_hashlib_hmac_get_name(PyObject *op, void *Py_UNUSED(closure))
18301846
{
18311847
HMACobject *self = HMACobject_CAST(op);
1832-
PyObject *digest_name = py_digest_name(HMAC_CTX_get_md(self->ctx));
1848+
const EVP_MD *md = HMAC_CTX_get_md(self->ctx);
1849+
if (md == NULL) {
1850+
return _setException(PyExc_ValueError, NULL);
1851+
}
1852+
PyObject *digest_name = py_digest_name(md);
18331853
if (digest_name == NULL) {
18341854
return NULL;
18351855
}
@@ -1980,7 +2000,7 @@ _hashlib_get_fips_mode_impl(PyObject *module)
19802000
// But 0 is also a valid result value.
19812001
unsigned long errcode = ERR_peek_last_error();
19822002
if (errcode) {
1983-
_setException(PyExc_ValueError, NULL);
2003+
(void)_setException(PyExc_ValueError, NULL);
19842004
return -1;
19852005
}
19862006
}

0 commit comments

Comments
 (0)