From fd378ad6ce722b11718d2e30af9a4edef393a0e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 25 Jan 2025 11:04:22 +0100 Subject: [PATCH 1/7] fix UBSan failures for `EVPobject` --- Modules/_hashopenssl.c | 55 ++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 32 deletions(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 082929be3c77b7..c2bbc228ae8c6b 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -280,6 +280,8 @@ typedef struct { PyMutex mutex; /* OpenSSL context lock */ } EVPobject; +#define _EVPobject_CAST(op) ((EVPobject *)(op)) + typedef struct { PyObject_HEAD HMAC_CTX *ctx; /* OpenSSL hmac context */ @@ -499,8 +501,9 @@ EVP_hash(EVPobject *self, const void *vp, Py_ssize_t len) /* Internal methods for a hash object */ static void -EVP_dealloc(EVPobject *self) +EVP_dealloc(PyObject *op) { + EVPobject *self = _EVPobject_CAST(op); PyTypeObject *tp = Py_TYPE(self); EVP_MD_CTX_free(self->ctx); PyObject_Free(self); @@ -658,55 +661,46 @@ static PyMethodDef EVP_methods[] = { }; static PyObject * -EVP_get_block_size(EVPobject *self, void *closure) +EVP_get_block_size(PyObject *op, void *Py_UNUSED(closure)) { - long block_size; - block_size = EVP_MD_CTX_block_size(self->ctx); + EVPobject *self = _EVPobject_CAST(op); + long block_size = EVP_MD_CTX_block_size(self->ctx); return PyLong_FromLong(block_size); } static PyObject * -EVP_get_digest_size(EVPobject *self, void *closure) +EVP_get_digest_size(PyObject *op, void *Py_UNUSED(closure)) { - long size; - size = EVP_MD_CTX_size(self->ctx); + EVPobject *self = _EVPobject_CAST(op); + long size = EVP_MD_CTX_size(self->ctx); return PyLong_FromLong(size); } static PyObject * -EVP_get_name(EVPobject *self, void *closure) +EVP_get_name(PyObject *op, void *Py_UNUSED(closure)) { + EVPobject *self = _EVPobject_CAST(op); return py_digest_name(EVP_MD_CTX_md(self->ctx)); } static PyGetSetDef EVP_getseters[] = { - {"digest_size", - (getter)EVP_get_digest_size, NULL, - NULL, - NULL}, - {"block_size", - (getter)EVP_get_block_size, NULL, - NULL, - NULL}, - {"name", - (getter)EVP_get_name, NULL, - NULL, - PyDoc_STR("algorithm name.")}, + {"digest_size", EVP_get_digest_size, NULL, NULL, NULL}, + {"block_size", EVP_get_block_size, NULL, NULL, NULL}, + {"name", EVP_get_name, NULL, NULL, PyDoc_STR("algorithm name.")}, {NULL} /* Sentinel */ }; static PyObject * -EVP_repr(EVPobject *self) +EVP_repr(PyObject *self) { - PyObject *name_obj, *repr; - name_obj = py_digest_name(EVP_MD_CTX_md(self->ctx)); - if (!name_obj) { + PyObject *name = EVP_get_name(self, NULL); + if (name == NULL) { return NULL; } - repr = PyUnicode_FromFormat("<%U %s object @ %p>", - name_obj, Py_TYPE(self)->tp_name, self); - Py_DECREF(name_obj); + PyObject *repr = PyUnicode_FromFormat("<%U %T object @ %p>", + name, self, self); + Py_DECREF(name); return repr; } @@ -848,16 +842,13 @@ static PyMethodDef EVPXOF_methods[] = { static PyObject * -EVPXOF_get_digest_size(EVPobject *self, void *closure) +EVPXOF_get_digest_size(PyObject *Py_UNUSED(self), void *Py_UNUSED(closure)) { return PyLong_FromLong(0); } static PyGetSetDef EVPXOF_getseters[] = { - {"digest_size", - (getter)EVPXOF_get_digest_size, NULL, - NULL, - NULL}, + {"digest_size", EVPXOF_get_digest_size, NULL, NULL, NULL}, {NULL} /* Sentinel */ }; From 77493a40fdc9177b399b03419e02ab92cc6bce68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 25 Jan 2025 11:17:14 +0100 Subject: [PATCH 2/7] fix UBSan failures for `HMACobject` --- Modules/_hashopenssl.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index c2bbc228ae8c6b..427326a28c3c7a 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -290,6 +290,8 @@ typedef struct { PyMutex mutex; /* HMAC context lock */ } HMACobject; +#define _HMACobject_CAST(op) ((HMACobject *)(op)) + #include "clinic/_hashopenssl.c.h" /*[clinic input] module _hashlib @@ -680,6 +682,7 @@ 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)); } @@ -1513,6 +1516,7 @@ _hashlib_hmac_singleshot_impl(PyObject *module, Py_buffer *key, */ static int _hmac_update(HMACobject*, PyObject*); +static PyObject *_hashlib_hmac_get_name(PyObject *, void *); /*[clinic input] _hashlib.hmac_new @@ -1605,6 +1609,7 @@ locked_HMAC_CTX_copy(HMAC_CTX *new_ctx_p, HMACobject *self) 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)); assert(digest_size <= EVP_MAX_MD_SIZE); return digest_size; @@ -1673,8 +1678,9 @@ _hashlib_HMAC_copy_impl(HMACobject *self) } static void -_hmac_dealloc(HMACobject *self) +_hmac_dealloc(PyObject *op) { + HMACobject *self = _HMACobject_CAST(op); PyTypeObject *tp = Py_TYPE(self); HMAC_CTX_free(self->ctx); PyObject_Free(self); @@ -1682,9 +1688,9 @@ _hmac_dealloc(HMACobject *self) } static PyObject * -_hmac_repr(HMACobject *self) +_hmac_repr(PyObject *self) { - PyObject *digest_name = py_digest_name(HMAC_CTX_get_md(self->ctx)); + PyObject *digest_name = _hashlib_hmac_get_name(self, NULL); if (digest_name == NULL) { return NULL; } @@ -1780,8 +1786,9 @@ _hashlib_HMAC_hexdigest_impl(HMACobject *self) } static PyObject * -_hashlib_hmac_get_digest_size(HMACobject *self, void *closure) +_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); @@ -1790,8 +1797,9 @@ _hashlib_hmac_get_digest_size(HMACobject *self, void *closure) } static PyObject * -_hashlib_hmac_get_block_size(HMACobject *self, void *closure) +_hashlib_hmac_get_block_size(PyObject *op, void *Py_UNUSED(closure)) { + HMACobject *self = _HMACobject_CAST(op); const EVP_MD *md = HMAC_CTX_get_md(self->ctx); if (md == NULL) { return _setException(PyExc_ValueError, NULL); @@ -1800,8 +1808,9 @@ _hashlib_hmac_get_block_size(HMACobject *self, void *closure) } static PyObject * -_hashlib_hmac_get_name(HMACobject *self, void *closure) +_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)); if (digest_name == NULL) { return NULL; @@ -1820,9 +1829,9 @@ static PyMethodDef HMAC_methods[] = { }; static PyGetSetDef HMAC_getset[] = { - {"digest_size", (getter)_hashlib_hmac_get_digest_size, NULL, NULL, NULL}, - {"block_size", (getter)_hashlib_hmac_get_block_size, NULL, NULL, NULL}, - {"name", (getter)_hashlib_hmac_get_name, NULL, NULL, NULL}, + {"digest_size", _hashlib_hmac_get_digest_size, NULL, NULL, NULL}, + {"block_size", _hashlib_hmac_get_block_size, NULL, NULL, NULL}, + {"name", _hashlib_hmac_get_name, NULL, NULL, NULL}, {NULL} /* Sentinel */ }; @@ -1844,8 +1853,8 @@ digest_size -- number of bytes in digest() output\n"); static PyType_Slot HMACtype_slots[] = { {Py_tp_doc, (char *)hmactype_doc}, - {Py_tp_repr, (reprfunc)_hmac_repr}, - {Py_tp_dealloc,(destructor)_hmac_dealloc}, + {Py_tp_repr, _hmac_repr}, + {Py_tp_dealloc, _hmac_dealloc}, {Py_tp_methods, HMAC_methods}, {Py_tp_getset, HMAC_getset}, {0, NULL} From 7c59c744e3fb3f30ca1badab2a7b79d04aa98bcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 25 Jan 2025 11:18:10 +0100 Subject: [PATCH 3/7] suppress unused return values --- Modules/_hashopenssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 427326a28c3c7a..4f22f854325dc6 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -2147,7 +2147,7 @@ hashlib_clear(PyObject *m) static void hashlib_free(void *m) { - hashlib_clear((PyObject *)m); + (void)hashlib_clear((PyObject *)m); } /* Py_mod_exec functions */ From 66cf1fb559f410db7fe88f341374ce7e2ce5647a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 25 Jan 2025 11:18:41 +0100 Subject: [PATCH 4/7] remove redundant casts --- Modules/_hashopenssl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 4f22f854325dc6..11416f6a304a41 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -464,7 +464,7 @@ py_digest_by_digestmod(PyObject *module, PyObject *digestmod, enum Py_hash_type static EVPobject * newEVPobject(PyTypeObject *type) { - EVPobject *retval = (EVPobject *)PyObject_New(EVPobject, type); + EVPobject *retval = PyObject_New(EVPobject, type); if (retval == NULL) { return NULL; } @@ -1574,7 +1574,7 @@ _hashlib_hmac_new_impl(PyObject *module, Py_buffer *key, PyObject *msg_obj, goto error; } - self = (HMACobject *)PyObject_New(HMACobject, type); + self = PyObject_New(HMACobject, type); if (self == NULL) { goto error; } @@ -1666,7 +1666,7 @@ _hashlib_HMAC_copy_impl(HMACobject *self) return _setException(PyExc_ValueError, NULL); } - retval = (HMACobject *)PyObject_New(HMACobject, Py_TYPE(self)); + retval = PyObject_New(HMACobject, Py_TYPE(self)); if (retval == NULL) { HMAC_CTX_free(ctx); return NULL; From b932b8333520aaf0eea6a34e939536bcead96297 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 8 Feb 2025 10:23:15 +0100 Subject: [PATCH 5/7] Do Do not use `_` + capital letter in new code as it is also UB. --- Modules/_hashopenssl.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 11416f6a304a41..009d7ab06466df 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -280,7 +280,7 @@ typedef struct { PyMutex mutex; /* OpenSSL context lock */ } EVPobject; -#define _EVPobject_CAST(op) ((EVPobject *)(op)) +#define EVPobject_CAST(op) ((EVPobject *)(op)) typedef struct { PyObject_HEAD @@ -290,7 +290,7 @@ typedef struct { PyMutex mutex; /* HMAC context lock */ } HMACobject; -#define _HMACobject_CAST(op) ((HMACobject *)(op)) +#define HMACobject_CAST(op) ((HMACobject *)(op)) #include "clinic/_hashopenssl.c.h" /*[clinic input] @@ -505,7 +505,7 @@ EVP_hash(EVPobject *self, const void *vp, Py_ssize_t len) static void EVP_dealloc(PyObject *op) { - EVPobject *self = _EVPobject_CAST(op); + EVPobject *self = EVPobject_CAST(op); PyTypeObject *tp = Py_TYPE(self); EVP_MD_CTX_free(self->ctx); PyObject_Free(self); @@ -665,7 +665,7 @@ static PyMethodDef EVP_methods[] = { static PyObject * EVP_get_block_size(PyObject *op, void *Py_UNUSED(closure)) { - EVPobject *self = _EVPobject_CAST(op); + EVPobject *self = EVPobject_CAST(op); long block_size = EVP_MD_CTX_block_size(self->ctx); return PyLong_FromLong(block_size); } @@ -673,7 +673,7 @@ EVP_get_block_size(PyObject *op, void *Py_UNUSED(closure)) static PyObject * EVP_get_digest_size(PyObject *op, void *Py_UNUSED(closure)) { - EVPobject *self = _EVPobject_CAST(op); + EVPobject *self = EVPobject_CAST(op); long size = EVP_MD_CTX_size(self->ctx); return PyLong_FromLong(size); } @@ -681,7 +681,7 @@ EVP_get_digest_size(PyObject *op, void *Py_UNUSED(closure)) static PyObject * EVP_get_name(PyObject *op, void *Py_UNUSED(closure)) { - EVPobject *self = _EVPobject_CAST(op); + 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)); } @@ -1680,7 +1680,7 @@ _hashlib_HMAC_copy_impl(HMACobject *self) static void _hmac_dealloc(PyObject *op) { - HMACobject *self = _HMACobject_CAST(op); + HMACobject *self = HMACobject_CAST(op); PyTypeObject *tp = Py_TYPE(self); HMAC_CTX_free(self->ctx); PyObject_Free(self); @@ -1788,7 +1788,7 @@ _hashlib_HMAC_hexdigest_impl(HMACobject *self) static PyObject * _hashlib_hmac_get_digest_size(PyObject *op, void *Py_UNUSED(closure)) { - HMACobject *self = _HMACobject_CAST(op); + HMACobject *self = HMACobject_CAST(op); unsigned int digest_size = _hmac_digest_size(self); if (digest_size == 0) { return _setException(PyExc_ValueError, NULL); @@ -1799,7 +1799,7 @@ _hashlib_hmac_get_digest_size(PyObject *op, void *Py_UNUSED(closure)) static PyObject * _hashlib_hmac_get_block_size(PyObject *op, void *Py_UNUSED(closure)) { - HMACobject *self = _HMACobject_CAST(op); + HMACobject *self = HMACobject_CAST(op); const EVP_MD *md = HMAC_CTX_get_md(self->ctx); if (md == NULL) { return _setException(PyExc_ValueError, NULL); @@ -1810,7 +1810,7 @@ _hashlib_hmac_get_block_size(PyObject *op, void *Py_UNUSED(closure)) static PyObject * _hashlib_hmac_get_name(PyObject *op, void *Py_UNUSED(closure)) { - HMACobject *self = _HMACobject_CAST(op); + HMACobject *self = HMACobject_CAST(op); PyObject *digest_name = py_digest_name(HMAC_CTX_get_md(self->ctx)); if (digest_name == NULL) { return NULL; From 26faaa59b44e07ab6e5eb227e505ddf08aacf5f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 25 Feb 2025 13:27:50 +0100 Subject: [PATCH 6/7] fix a bad representation! --- Modules/_hashopenssl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 2b699c37cd85a3..9fb3e1be735ec7 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -1706,9 +1706,10 @@ _hmac_dealloc(PyObject *op) } static PyObject * -_hmac_repr(PyObject *self) +_hmac_repr(PyObject *op) { - PyObject *digest_name = _hashlib_hmac_get_name(self, NULL); + HMACobject *self = HMACobject_CAST(op); + PyObject *digest_name = py_digest_name(HMAC_CTX_get_md(self->ctx)); if (digest_name == NULL) { return NULL; } From 30e0c9b237afe4544b13c6956a37d497c7aaa3a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 25 Feb 2025 13:37:37 +0100 Subject: [PATCH 7/7] remove unnecessary forward declaration --- Modules/_hashopenssl.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 9fb3e1be735ec7..19609337479854 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -1534,7 +1534,6 @@ _hashlib_hmac_singleshot_impl(PyObject *module, Py_buffer *key, */ static int _hmac_update(HMACobject*, PyObject*); -static PyObject *_hashlib_hmac_get_name(PyObject *, void *); /*[clinic input] _hashlib.hmac_new