gh-97616: list_resize() checks for integer overflow

[vstinner]

Fix multiplying a list by an integer (list * int): detect the integer overflow when the new allocated length is close to the maximum size. Issue reported by Jordan Limor.

list_resize() now checks for integer overflow before multiplying the new allocated length by the list item size (sizeof(PyObject*)).

• Issue: [security] list_resize(): integer overflow if newsize is too large #97616

[vstinner]

@gpshead @serhiy-storchaka @methane: Would you mind to review my list_resize() fix?

[serhiy-storchaka]

There is no integer overflow here.

[serhiy-storchaka]

Or rather there should not be integer overflow. Unsigned sizes are intentionally used to avoid integer overflow.

[gpshead]

There is no integer overflow here.

Clarification: are you suggesting there was never a problem in the first place or that this change looks good?

A sufficiently large newsize previously would have caused num_allocated_bytes passed to realloc to be too small.

[serhiy-storchaka]

At first glance, I saw that the existing code was already written to protect against integer overflow, so an explicit check seemed redundant. Then I read the issue and saw that it only protected against some cases of integer overflows. So the code in this PR is correct and necessary.

[vstinner]

See #97616 (comment) for an explanation of the integer overflow.

[vstinner]

Explicitly cast PY_SSIZE_T_MAX to size_t

I hate implicit conversion between signed and unsigned, it commonly emits compiler warnings. I prefer to only work on unsigned numbers there.

[vstinner]

I ran the added test on the main branch: it does crash as expected.

vstinner@mona$ ./python -m test -v test_list -m test_list_resize_overflow
...
test_list_resize_overflow (test.test_list.ListTest.test_list_resize_overflow) ... Fatal Python error: Segmentation fault

Current thread 0x00007faeb98da740 (most recent call first):
  File "/home/vstinner/python/main/Lib/test/test_list.py", line 110 in test_list_resize_overflow
  ...

It only crashs on the second test (lst *= size): tuple.__imul__() works in-place and so calls list_resize().

        size = ((2 ** (tuple.__itemsize__ * 8) - 1) // 2)
        with self.assertRaises((MemoryError, OverflowError)):
            lst * size
        with self.assertRaises((MemoryError, OverflowError)):
            lst *= size   # <=== HERE

The first one (lst * size) doesn't crash: tuple.__mul__() creates a new list, it doesn't call list_resize().

I prefer to test both cases.

[vstinner]

Fix multiplying a list by an integer (list * int)

Well, the bug is on list *= int, I fixed the doc.

[miss-islington]

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8, 3.9, 3.10, 3.11.
🐍🍒⛏🤖

[bedevere-bot]

GH-97625 is a backport of this pull request to the 3.11 branch.

[bedevere-bot]

GH-97626 is a backport of this pull request to the 3.10 branch.

[bedevere-bot]

GH-97627 is a backport of this pull request to the 3.9 branch.

[bedevere-bot]

GH-97628 is a backport of this pull request to the 3.8 branch.

[bedevere-bot]

GH-97629 is a backport of this pull request to the 3.7 branch.

[lazka]

If such overflow checks are common in CPython you could consider using macros like in glib for example: https://developer-old.gnome.org/glib/stable/glib-Bounds-checked-integer-arithmetic.html