Skip to content

Commit d968baa

Browse files
committed
reject negative sizes in SHAKE digests
1 parent 59963e8 commit d968baa

File tree

5 files changed

+113
-39
lines changed

5 files changed

+113
-39
lines changed

Lib/test/test_hashlib.py

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,11 @@ def hash_constructors(self):
207207
constructors = self.constructors_to_test.values()
208208
return itertools.chain.from_iterable(constructors)
209209

210+
@property
211+
def shake_constructors(self):
212+
for shake_name in self.shakes:
213+
yield from self.constructors_to_test.get(shake_name, ())
214+
210215
@property
211216
def is_fips_mode(self):
212217
return get_fips_mode()
@@ -376,21 +381,50 @@ def test_hexdigest(self):
376381
self.assertIsInstance(h.digest(), bytes)
377382
self.assertEqual(hexstr(h.digest()), h.hexdigest())
378383

379-
def test_digest_length_overflow(self):
380-
# See issue #34922
381-
large_sizes = (2**29, 2**32-10, 2**32+10, 2**61, 2**64-10, 2**64+10)
382-
for cons in self.hash_constructors:
383-
h = cons(usedforsecurity=False)
384-
if h.name not in self.shakes:
385-
continue
386-
if HASH is not None and isinstance(h, HASH):
387-
# _hashopenssl's take a size_t
388-
continue
389-
for digest in h.digest, h.hexdigest:
390-
self.assertRaises(ValueError, digest, -10)
391-
for length in large_sizes:
392-
with self.assertRaises((ValueError, OverflowError)):
393-
digest(length)
384+
def test_shakes_zero_digest_length(self):
385+
for constructor in self.shake_constructors:
386+
with self.subTest(constructor=constructor):
387+
h = constructor(b'abcdef', usedforsecurity=False)
388+
self.assertEqual(h.digest(0), b'')
389+
self.assertEqual(h.hexdigest(0), '')
390+
391+
def test_shakes_invalid_digest_length(self):
392+
# See https://github.com/python/cpython/issues/79103.
393+
for constructor in self.shake_constructors:
394+
with self.subTest(constructor=constructor):
395+
h = constructor(usedforsecurity=False)
396+
# Note: digest() and hexdigest() take a signed input and
397+
# raise if it is negative; the rationale is that we use
398+
# internally PyBytes_FromStringAndSize() and _Py_strhex()
399+
# which both take a Py_ssize_t.
400+
for negative_size in (-1, -10, -(1 << 31), -sys.maxsize):
401+
self.assertRaises(ValueError, h.digest, negative_size)
402+
self.assertRaises(ValueError, h.hexdigest, negative_size)
403+
404+
def test_shakes_overflow_digest_length(self):
405+
# See https://github.com/python/cpython/issues/135759.
406+
407+
exc_types = (OverflowError, ValueError)
408+
# HACL* accepts an 'uint32_t' while OpenSSL accepts a 'size_t'.
409+
openssl_overflown_sizes = (sys.maxsize + 1, 2 * sys.maxsize)
410+
# https://github.com/python/cpython/issues/79103 restricts
411+
# the accepted built-in lengths to 2 ** 29, even if OpenSSL
412+
# accepts such lengths.
413+
builtin_overflown_sizes = openssl_overflown_sizes + (
414+
2 ** 29, 2 ** 32 - 10, 2 ** 32, 2 ** 32 + 10,
415+
2 ** 61, 2 ** 64 - 10, 2 ** 64, 2 ** 64 + 10,
416+
)
417+
418+
for constructor in self.shake_constructors:
419+
with self.subTest(constructor=constructor):
420+
h = constructor(usedforsecurity=False)
421+
if HASH is not None and isinstance(h, HASH):
422+
overflown_sizes = openssl_overflown_sizes
423+
else:
424+
overflown_sizes = builtin_overflown_sizes
425+
for invalid_size in overflown_sizes:
426+
self.assertRaises(exc_types, h.digest, invalid_size)
427+
self.assertRaises(exc_types, h.hexdigest, invalid_size)
394428

395429
def test_name_attribute(self):
396430
for cons in self.hash_constructors:
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
:mod:`hashlib`: correctly reject negative digest lengths in SHAKE objects.
2+
Patch by Bénédikt Tran.

Modules/_hashopenssl.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -938,8 +938,14 @@ _hashlib_HASHXOF_digest_impl(HASHobject *self, Py_ssize_t length)
938938
/*[clinic end generated code: output=dcb09335dd2fe908 input=3eb034ce03c55b21]*/
939939
{
940940
EVP_MD_CTX *temp_ctx;
941-
PyObject *retval = PyBytes_FromStringAndSize(NULL, length);
941+
PyObject *retval;
942+
943+
if (length < 0) {
944+
PyErr_SetString(PyExc_ValueError, "negative digest length");
945+
return NULL;
946+
}
942947

948+
retval = PyBytes_FromStringAndSize(NULL, length);
943949
if (retval == NULL) {
944950
return NULL;
945951
}
@@ -986,9 +992,14 @@ _hashlib_HASHXOF_hexdigest_impl(HASHobject *self, Py_ssize_t length)
986992
EVP_MD_CTX *temp_ctx;
987993
PyObject *retval;
988994

995+
if (length < 0) {
996+
PyErr_SetString(PyExc_ValueError, "negative digest length");
997+
return NULL;
998+
}
999+
9891000
digest = (unsigned char*)PyMem_Malloc(length);
9901001
if (digest == NULL) {
991-
PyErr_NoMemory();
1002+
(void)PyErr_NoMemory();
9921003
return NULL;
9931004
}
9941005

Modules/clinic/sha3module.c.h

Lines changed: 28 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Modules/sha3module.c

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -473,16 +473,25 @@ SHA3_TYPE_SLOTS(sha3_512_slots, sha3_512__doc__, SHA3_methods, SHA3_getseters);
473473
SHA3_TYPE_SPEC(sha3_512_spec, "sha3_512", sha3_512_slots);
474474

475475
static PyObject *
476-
_SHAKE_digest(PyObject *op, unsigned long digestlen, int hex)
476+
_SHAKE_digest(SHA3object *self, Py_ssize_t digestlen, int hex)
477477
{
478478
unsigned char *digest = NULL;
479479
PyObject *result = NULL;
480-
SHA3object *self = _SHA3object_CAST(op);
481480

482-
if (digestlen >= (1 << 29)) {
483-
PyErr_SetString(PyExc_ValueError, "length is too large");
481+
if (digestlen < 0) {
482+
PyErr_SetString(PyExc_ValueError, "negative digest length");
483+
return NULL;
484+
}
485+
if ((size_t)digestlen >= (1 << 29)) {
486+
/*
487+
* Raise OverflowError to match the semantics of OpenSSL SHAKE
488+
* when the digest length exceeds the range of a 'Py_ssize_t';
489+
* the exception message will however be different in this case.
490+
*/
491+
PyErr_SetString(PyExc_OverflowError, "digest length is too large");
484492
return NULL;
485493
}
494+
486495
digest = (unsigned char*)PyMem_Malloc(digestlen);
487496
if (digest == NULL) {
488497
return PyErr_NoMemory();
@@ -509,32 +518,32 @@ _SHAKE_digest(PyObject *op, unsigned long digestlen, int hex)
509518
/*[clinic input]
510519
_sha3.shake_128.digest
511520
512-
length: unsigned_long
521+
length: Py_ssize_t
513522
514523
Return the digest value as a bytes object.
515524
[clinic start generated code]*/
516525

517526
static PyObject *
518-
_sha3_shake_128_digest_impl(SHA3object *self, unsigned long length)
519-
/*[clinic end generated code: output=2313605e2f87bb8f input=93d6d6ff32904f18]*/
527+
_sha3_shake_128_digest_impl(SHA3object *self, Py_ssize_t length)
528+
/*[clinic end generated code: output=6c53fb71a6cff0a0 input=be03ade4b31dd54c]*/
520529
{
521-
return _SHAKE_digest((PyObject *)self, length, 0);
530+
return _SHAKE_digest(self, length, 0);
522531
}
523532

524533

525534
/*[clinic input]
526535
_sha3.shake_128.hexdigest
527536
528-
length: unsigned_long
537+
length: Py_ssize_t
529538
530539
Return the digest value as a string of hexadecimal digits.
531540
[clinic start generated code]*/
532541

533542
static PyObject *
534-
_sha3_shake_128_hexdigest_impl(SHA3object *self, unsigned long length)
535-
/*[clinic end generated code: output=bf8e2f1e490944a8 input=562d74e7060b56ab]*/
543+
_sha3_shake_128_hexdigest_impl(SHA3object *self, Py_ssize_t length)
544+
/*[clinic end generated code: output=a27412d404f64512 input=0d84d05d7a8ccd37]*/
536545
{
537-
return _SHAKE_digest((PyObject *)self, length, 1);
546+
return _SHAKE_digest(self, length, 1);
538547
}
539548

540549
static PyObject *

0 commit comments

Comments
 (0)