gh-135759: consistently reject negative sizes in SHAKE digests (#135767)
Some checks are pending
Tests / (push) Blocked by required conditions
Tests / Windows MSI (push) Blocked by required conditions
Tests / Docs (push) Blocked by required conditions
Tests / WASI (push) Blocked by required conditions
Tests / Hypothesis tests on Ubuntu (push) Blocked by required conditions
Tests / Check if Autoconf files are up to date (push) Blocked by required conditions
Tests / Change detection (push) Waiting to run
Tests / Check if generated files are up to date (push) Blocked by required conditions
Tests / Ubuntu SSL tests with OpenSSL (push) Blocked by required conditions
Tests / Address sanitizer (push) Blocked by required conditions
Tests / Undefined behavior sanitizer (push) Blocked by required conditions
Tests / Cross build Linux (push) Blocked by required conditions
Tests / CIFuzz (push) Blocked by required conditions
Tests / All required checks pass (push) Blocked by required conditions
Lint / lint (push) Waiting to run
mypy / Run mypy on Lib/_pyrepl (push) Waiting to run
mypy / Run mypy on Lib/test/libregrtest (push) Waiting to run
mypy / Run mypy on Lib/tomllib (push) Waiting to run
mypy / Run mypy on Tools/build (push) Waiting to run
mypy / Run mypy on Tools/cases_generator (push) Waiting to run
mypy / Run mypy on Tools/clinic (push) Waiting to run
mypy / Run mypy on Tools/jit (push) Waiting to run
mypy / Run mypy on Tools/peg_generator (push) Waiting to run

Passing a negative digest length to `_hashilb.HASHXOF.[hex]digest()` now
raises a ValueError instead of a MemoryError or a SystemError. This makes
the behavior consistent with that of `_sha3.shake_{128,256}.[hex]digest`.
This commit is contained in:
Bénédikt Tran 2025-06-21 11:43:30 +02:00 committed by GitHub
parent 13cac83347
commit 7c4361564c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 120 additions and 40 deletions

View file

@ -207,6 +207,11 @@ class HashLibTestCase(unittest.TestCase):
constructors = self.constructors_to_test.values()
return itertools.chain.from_iterable(constructors)
@property
def shake_constructors(self):
for shake_name in self.shakes:
yield from self.constructors_to_test.get(shake_name, ())
@property
def is_fips_mode(self):
return get_fips_mode()
@ -376,21 +381,50 @@ class HashLibTestCase(unittest.TestCase):
self.assertIsInstance(h.digest(), bytes)
self.assertEqual(hexstr(h.digest()), h.hexdigest())
def test_digest_length_overflow(self):
# See issue #34922
large_sizes = (2**29, 2**32-10, 2**32+10, 2**61, 2**64-10, 2**64+10)
for cons in self.hash_constructors:
h = cons(usedforsecurity=False)
if h.name not in self.shakes:
continue
if HASH is not None and isinstance(h, HASH):
# _hashopenssl's take a size_t
continue
for digest in h.digest, h.hexdigest:
self.assertRaises(ValueError, digest, -10)
for length in large_sizes:
with self.assertRaises((ValueError, OverflowError)):
digest(length)
def test_shakes_zero_digest_length(self):
for constructor in self.shake_constructors:
with self.subTest(constructor=constructor):
h = constructor(b'abcdef', usedforsecurity=False)
self.assertEqual(h.digest(0), b'')
self.assertEqual(h.hexdigest(0), '')
def test_shakes_invalid_digest_length(self):
# See https://github.com/python/cpython/issues/79103.
for constructor in self.shake_constructors:
with self.subTest(constructor=constructor):
h = constructor(usedforsecurity=False)
# Note: digest() and hexdigest() take a signed input and
# raise if it is negative; the rationale is that we use
# internally PyBytes_FromStringAndSize() and _Py_strhex()
# which both take a Py_ssize_t.
for negative_size in (-1, -10, -(1 << 31), -sys.maxsize):
self.assertRaises(ValueError, h.digest, negative_size)
self.assertRaises(ValueError, h.hexdigest, negative_size)
def test_shakes_overflow_digest_length(self):
# See https://github.com/python/cpython/issues/135759.
exc_types = (OverflowError, ValueError)
# HACL* accepts an 'uint32_t' while OpenSSL accepts a 'size_t'.
openssl_overflown_sizes = (sys.maxsize + 1, 2 * sys.maxsize)
# https://github.com/python/cpython/issues/79103 restricts
# the accepted built-in lengths to 2 ** 29, even if OpenSSL
# accepts such lengths.
builtin_overflown_sizes = openssl_overflown_sizes + (
2 ** 29, 2 ** 32 - 10, 2 ** 32, 2 ** 32 + 10,
2 ** 61, 2 ** 64 - 10, 2 ** 64, 2 ** 64 + 10,
)
for constructor in self.shake_constructors:
with self.subTest(constructor=constructor):
h = constructor(usedforsecurity=False)
if HASH is not None and isinstance(h, HASH):
overflown_sizes = openssl_overflown_sizes
else:
overflown_sizes = builtin_overflown_sizes
for invalid_size in overflown_sizes:
self.assertRaises(exc_types, h.digest, invalid_size)
self.assertRaises(exc_types, h.hexdigest, invalid_size)
def test_name_attribute(self):
for cons in self.hash_constructors:

View file

@ -0,0 +1,4 @@
:mod:`hashlib`: reject negative digest lengths in OpenSSL-based SHAKE objects
by raising a :exc:`ValueError`. Previously, negative lengths were implicitly
rejected by raising a :exc:`MemoryError` or a :exc:`SystemError`.
Patch by Bénédikt Tran.

View file

@ -938,8 +938,14 @@ _hashlib_HASHXOF_digest_impl(HASHobject *self, Py_ssize_t length)
/*[clinic end generated code: output=dcb09335dd2fe908 input=3eb034ce03c55b21]*/
{
EVP_MD_CTX *temp_ctx;
PyObject *retval = PyBytes_FromStringAndSize(NULL, length);
PyObject *retval;
if (length < 0) {
PyErr_SetString(PyExc_ValueError, "negative digest length");
return NULL;
}
retval = PyBytes_FromStringAndSize(NULL, length);
if (retval == NULL) {
return NULL;
}
@ -986,9 +992,14 @@ _hashlib_HASHXOF_hexdigest_impl(HASHobject *self, Py_ssize_t length)
EVP_MD_CTX *temp_ctx;
PyObject *retval;
if (length < 0) {
PyErr_SetString(PyExc_ValueError, "negative digest length");
return NULL;
}
digest = (unsigned char*)PyMem_Malloc(length);
if (digest == NULL) {
PyErr_NoMemory();
(void)PyErr_NoMemory();
return NULL;
}

View file

@ -6,7 +6,7 @@ preserve
# include "pycore_gc.h" // PyGC_Head
# include "pycore_runtime.h" // _Py_ID()
#endif
#include "pycore_long.h" // _PyLong_UnsignedLong_Converter()
#include "pycore_abstract.h" // _PyNumber_Index()
#include "pycore_modsupport.h" // _PyArg_UnpackKeywords()
PyDoc_STRVAR(py_sha3_new__doc__,
@ -179,7 +179,7 @@ PyDoc_STRVAR(_sha3_shake_128_digest__doc__,
{"digest", _PyCFunction_CAST(_sha3_shake_128_digest), METH_FASTCALL|METH_KEYWORDS, _sha3_shake_128_digest__doc__},
static PyObject *
_sha3_shake_128_digest_impl(SHA3object *self, unsigned long length);
_sha3_shake_128_digest_impl(SHA3object *self, Py_ssize_t length);
static PyObject *
_sha3_shake_128_digest(PyObject *self, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames)
@ -213,15 +213,24 @@ _sha3_shake_128_digest(PyObject *self, PyObject *const *args, Py_ssize_t nargs,
};
#undef KWTUPLE
PyObject *argsbuf[1];
unsigned long length;
Py_ssize_t length;
args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser,
/*minpos*/ 1, /*maxpos*/ 1, /*minkw*/ 0, /*varpos*/ 0, argsbuf);
if (!args) {
goto exit;
}
if (!_PyLong_UnsignedLong_Converter(args[0], &length)) {
goto exit;
{
Py_ssize_t ival = -1;
PyObject *iobj = _PyNumber_Index(args[0]);
if (iobj != NULL) {
ival = PyLong_AsSsize_t(iobj);
Py_DECREF(iobj);
}
if (ival == -1 && PyErr_Occurred()) {
goto exit;
}
length = ival;
}
return_value = _sha3_shake_128_digest_impl((SHA3object *)self, length);
@ -239,7 +248,7 @@ PyDoc_STRVAR(_sha3_shake_128_hexdigest__doc__,
{"hexdigest", _PyCFunction_CAST(_sha3_shake_128_hexdigest), METH_FASTCALL|METH_KEYWORDS, _sha3_shake_128_hexdigest__doc__},
static PyObject *
_sha3_shake_128_hexdigest_impl(SHA3object *self, unsigned long length);
_sha3_shake_128_hexdigest_impl(SHA3object *self, Py_ssize_t length);
static PyObject *
_sha3_shake_128_hexdigest(PyObject *self, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames)
@ -273,19 +282,28 @@ _sha3_shake_128_hexdigest(PyObject *self, PyObject *const *args, Py_ssize_t narg
};
#undef KWTUPLE
PyObject *argsbuf[1];
unsigned long length;
Py_ssize_t length;
args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser,
/*minpos*/ 1, /*maxpos*/ 1, /*minkw*/ 0, /*varpos*/ 0, argsbuf);
if (!args) {
goto exit;
}
if (!_PyLong_UnsignedLong_Converter(args[0], &length)) {
goto exit;
{
Py_ssize_t ival = -1;
PyObject *iobj = _PyNumber_Index(args[0]);
if (iobj != NULL) {
ival = PyLong_AsSsize_t(iobj);
Py_DECREF(iobj);
}
if (ival == -1 && PyErr_Occurred()) {
goto exit;
}
length = ival;
}
return_value = _sha3_shake_128_hexdigest_impl((SHA3object *)self, length);
exit:
return return_value;
}
/*[clinic end generated code: output=65e437799472b89f input=a9049054013a1b77]*/
/*[clinic end generated code: output=c17e3ec670afe253 input=a9049054013a1b77]*/

View file

@ -473,16 +473,25 @@ SHA3_TYPE_SLOTS(sha3_512_slots, sha3_512__doc__, SHA3_methods, SHA3_getseters);
SHA3_TYPE_SPEC(sha3_512_spec, "sha3_512", sha3_512_slots);
static PyObject *
_SHAKE_digest(PyObject *op, unsigned long digestlen, int hex)
_SHAKE_digest(SHA3object *self, Py_ssize_t digestlen, int hex)
{
unsigned char *digest = NULL;
PyObject *result = NULL;
SHA3object *self = _SHA3object_CAST(op);
if (digestlen >= (1 << 29)) {
PyErr_SetString(PyExc_ValueError, "length is too large");
if (digestlen < 0) {
PyErr_SetString(PyExc_ValueError, "negative digest length");
return NULL;
}
if ((size_t)digestlen >= (1 << 29)) {
/*
* Raise OverflowError to match the semantics of OpenSSL SHAKE
* when the digest length exceeds the range of a 'Py_ssize_t';
* the exception message will however be different in this case.
*/
PyErr_SetString(PyExc_OverflowError, "digest length is too large");
return NULL;
}
digest = (unsigned char*)PyMem_Malloc(digestlen);
if (digest == NULL) {
return PyErr_NoMemory();
@ -493,7 +502,11 @@ _SHAKE_digest(PyObject *op, unsigned long digestlen, int hex)
* - the output length is zero -- we follow the existing behavior and return
* an empty digest, without raising an error */
if (digestlen > 0) {
(void)Hacl_Hash_SHA3_squeeze(self->hash_state, digest, digestlen);
#if PY_SSIZE_T_MAX > UINT32_MAX
assert(digestlen <= (Py_ssize_t)UINT32_MAX);
#endif
(void)Hacl_Hash_SHA3_squeeze(self->hash_state, digest,
(uint32_t)digestlen);
}
if (hex) {
result = _Py_strhex((const char *)digest, digestlen);
@ -509,32 +522,32 @@ _SHAKE_digest(PyObject *op, unsigned long digestlen, int hex)
/*[clinic input]
_sha3.shake_128.digest
length: unsigned_long
length: Py_ssize_t
Return the digest value as a bytes object.
[clinic start generated code]*/
static PyObject *
_sha3_shake_128_digest_impl(SHA3object *self, unsigned long length)
/*[clinic end generated code: output=2313605e2f87bb8f input=93d6d6ff32904f18]*/
_sha3_shake_128_digest_impl(SHA3object *self, Py_ssize_t length)
/*[clinic end generated code: output=6c53fb71a6cff0a0 input=be03ade4b31dd54c]*/
{
return _SHAKE_digest((PyObject *)self, length, 0);
return _SHAKE_digest(self, length, 0);
}
/*[clinic input]
_sha3.shake_128.hexdigest
length: unsigned_long
length: Py_ssize_t
Return the digest value as a string of hexadecimal digits.
[clinic start generated code]*/
static PyObject *
_sha3_shake_128_hexdigest_impl(SHA3object *self, unsigned long length)
/*[clinic end generated code: output=bf8e2f1e490944a8 input=562d74e7060b56ab]*/
_sha3_shake_128_hexdigest_impl(SHA3object *self, Py_ssize_t length)
/*[clinic end generated code: output=a27412d404f64512 input=0d84d05d7a8ccd37]*/
{
return _SHAKE_digest((PyObject *)self, length, 1);
return _SHAKE_digest(self, length, 1);
}
static PyObject *