gh-95778: CVE-2020-10735: Prevent DoS by very large int() #96499

gpshead · 2022-09-02T04:26:22Z

Integer to and from text conversions via CPython's bignum int type is not safe against denial of service attacks due to malicious input. Very large input strings with hundred thousands of digits can consume several CPU seconds.

This PR comes fresh from a pile of work done in our private PSRT security response team repo.

Signed-off-by: Christian Heimes [Red Hat] christian@python.org
Tons-of-polishing-up-by: Gregory P. Smith [Google] greg@krypto.org
Reviews via the private PSRT repo via many others (see the NEWS entry in the PR).

Issue: CVE-2020-10735: Prevent DoS by large int<->str conversions #95778

I wrote up a one pager for the release managers. Much of that text wound up in the Issue. Backports PRs already exist. See the issue for links.

Further Discussion

... is taking place in discuss.python.org threads

remaining TODOs (aka project management)

After this ships: file a follow-up issue to move int_max_str_digits into PyConfig in 3.12.
After this ships: file a feature request to add PySys_Audit hook calls at the new ValueError raise spots.
If this ships in 3.11rc2: file an issue to update the version notes about it in the 3.12 docs to say 3.11 and remove the 3.12 whatsnew text.
The above are tracked in main branch code cleanups for int_max_str_digits non-backportable TODOs #96512.
RedHat's CNA reserved that CVE number, they control interaction and coordination with MITRE on marking it public and assigning the score. - @tiran
assign CVSS score
- Assuming worst case, we they'll likely assign CVSS score 7.5 (High) to the issue, https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H&version=3.1

tiran · 2022-09-02T05:02:44Z

Closing and re-opening ticket. CI isn't starting.

The text to int parser of Python's int type is not safe against malicious input. Very large input strings with hundred thousands of digits can consume several seconds. The int() now limit the maximum amount of an input string to 5,000 digits. For comparison total amount of protons in the observable universe is known as Eddington number. That number has 80 digits. Signed-off-by: Christian Heimes <christian@python.org>

…e json module. "It's advised to limit the input to a sensible length." isn't very helpful for the JSON module as, while technically true, the limit needed to avoid hitting things like the int<->str base 10 conversion this issue is about would make a significant percentage of actual JSON used in the real world impractical. we're limiting the int/str conversion length, that is the best that can be done here - unless someone wants to implement a feature request for JSON to reject int looking fields at a much smaller base10 length limit.

The lumps way too many changes together in one commit, but extricating them into a series of individual pieces doesn't seem worthwhile at this stage, it could be done by picking and choosing into a new branch if there is a reason to do so. Summarizing off the top of my head after rereading diffs too many times: - Renamed the environment variable, -X flag, and names of the new APIs. - Cleaned up the documentation. - Refactored and improved some tests. - Moved the definitions of the default and threshold to a non-public header. - Set the default to 2000 instead of disabled as the Steering Council agreed having a default limit made the most sense for our users. - Lowered the minimum threshold because there seems no harm in allowing people to have a lower limit. - Fixed the other-bases base-10 digits equivalent estimate to be consistent with the actual potential base-10 value instead of wildly off. Aimed for consistency of number size here rather than any attempt to match the performance between bases as that is easier for users to understand and doesn't change if our algorithm implementations change. - Left a couple of notes about what changes look good or not good for a security fix backported into a stable release. It appears the PyConfig struct is a public API so we cannot change its definition mid-release. Backporting is going to involve something outside of the normal code path wise to plumb the new value through. Ugh. Questions: * [ ] `test_embed` is failing in an odd manner... what's up with that? * [ ] Is `_pydecimal` important? This change makes some of its APIs not work based on the values being used. The unittest suite that tests both the C and Python Decimal implementations revealed this and needed a higher limit set. If it relies on ints for arbitrary precision Decimal numbers, limits are frequently going to make it practically unusuable. * [ ] Backporting... we should produce what we believe a 3.11/3.10/3.9 backport will look like in its own PR branch before making a final decision. The most annoying difficulty being the `struct PyConfig` issue. * [ ] I pondered having values >0 but <threshold just silently be rounded up to the threshold value instead of causing an error at Python startup time, but we do have other environment variables and flags that cause startup time errors based on their value so it doesn't seem to matter either way. If we did that, the setter should also do that instead of raising.

Renamed to `int_max_str_digits` and simplified the logic per @Y1hgs's comments on the earlier revision. Less code, less awkward, and simpler to explain. Underscores and the sign are uncounted because that makes for the easiest implementation.

Playing it safe, if this lands in 3.11 before 3.11.0 these can be updated to 3.11.

zooba

Approved with typo fix (as pointed out by Nathaniel on one of the backports)

Doc/library/stdtypes.rst

Doc/whatsnew/3.12.rst

Lib/test/test_ast.py

Lib/test/test_compile.py

Misc/NEWS.d/next/Security/2022-08-07-16-53-38.gh-issue-95778.ch010gps.rst

Parser/pegen.c

Doc/library/stdtypes.rst

Co-authored-by: Steve Dower <steve.dower@microsoft.com>

gpshead · 2022-09-02T15:43:26Z

LOL at least I managed a consistent misspelling?

Lib/idlelib/idle_test/test_sidebar.py

) Integer to and from text conversions via CPython's bignum `int` type is not safe against denial of service attacks due to malicious input. Very large input strings with hundred thousands of digits can consume several CPU seconds. This PR comes fresh from a pile of work done in our private PSRT security response team repo. This backports #96499 aka 511ca94 Signed-off-by: Christian Heimes [Red Hat] <christian@python.org> Tons-of-polishing-up-by: Gregory P. Smith [Google] <greg@krypto.org> Reviews via the private PSRT repo via many others (see the NEWS entry in the PR).  * Issue: gh-95778  I wrote up [a one pager for the release managers](https://docs.google.com/document/d/1KjuF_aXlzPUxTK4BMgezGJ2Pn7uevfX7g0_mvgHlL7Y/edit#).

Doc/library/stdtypes.rst

Objects/longobject.c

light-bringer

good changes

Python/initconfig.c

septatrix · 2022-09-12T20:18:07Z

@gpshead could you maybe link to the respective discussion thread (https://discuss.python.org/t/int-str-conversions-broken-in-latest-python-bugfix-releases/18889/45) directly in the issue? It is already locked and you referred to https://discuss.python.org but given that the linked thread seems to be where the main discussion is happening it might be a bit more accessible and reduce duplicate threads about the same topic.

gpshead · 2022-09-13T00:29:19Z

good idea, done, link added.

ngie-eign · 2022-10-11T18:21:46Z

For others that might be curious, the testcases provided in the PR don't repro the issue on 2.7. I'm not sure about older 3.x versions.
PS I know python 2.7 is unsupported by python.org, but some OS distributions are still using python 2.7 in the field in a supported capacity.

/ pythongh-95800 Co-authored-by: Gregory P. Smith <gps@google.com>

/ pythongh-93306 Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>

…main (#98344) Add int/str security change from issue gh-95778 PRs gh-96499 / gh-95800 Co-authored-by: Gregory P. Smith <greg@krypto.org> [Google]

Summary: cherry-picked the upstream 3.10 backport ``` git cherry-pick 8f0fa4b eace09e ``` this one is python/cpython#96537 original commit message below -------- Integer to and from text conversions via CPython's bignum `int` type is not safe against denial of service attacks due to malicious input. Very large input strings with hundred thousands of digits can consume several CPU seconds. This PR comes fresh from a pile of work done in our private PSRT security response team repo. This backports python/cpython#96499 aka 511ca94 Signed-off-by: Christian Heimes [Red Hat] <christian@python.org> Tons-of-polishing-up-by: Gregory P. Smith [Google] <greg@krypto.org> Reviews via the private PSRT repo via many others (see the NEWS entry in the PR).  * Issue: gh-95778  I wrote up [a one pager for the release managers](https://docs.google.com/document/d/1KjuF_aXlzPUxTK4BMgezGJ2Pn7uevfX7g0_mvgHlL7Y/edit#). Reviewed By: alexmalyshev Differential Revision: D39369518 fbshipit-source-id: 6ed2def

bedevere-bot added the awaiting core review label Sep 2, 2022

gpshead self-assigned this Sep 2, 2022

gpshead added the release-blocker label Sep 2, 2022

gpshead assigned Yhg1s Sep 2, 2022

tiran closed this Sep 2, 2022

tiran reopened this Sep 2, 2022

tiran and others added 18 commits September 1, 2022 22:21

Default to disable, improve tests and docs

0a96b20

fix typo

88f6d5d

More docs (WIP)

70c195e

Basic documentation for sys functions

e17e93b

Use ValueError, ignore underscore, scale limit

fbd14b7

Fix CI

dd74d70

Address Greg's review

0e01461

Fix sys.flags len and docs

0b21e5f

Improve the configuring docs.

c90b79f

Remove the added now-unneeded helper log tbl fn.

ac9f22f

prevent intdostimeit from emitting errors in test_tools.

da72dd1

Remove a leftover base 10 reference. clarify.

d7e4d7b

versionadded/changed to 3.12

5c7e6d5

Playing it safe, if this lands in 3.11 before 3.11.0 these can be updated to 3.11.

Link to the CVE from the main doc.

61a5bc9

zooba approved these changes Sep 2, 2022

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Sep 2, 2022

tiran reviewed Sep 2, 2022

View reviewed changes

Doc/library/stdtypes.rst Outdated Show resolved Hide resolved

tiran and others added 2 commits September 2, 2022 16:55

Hexi hexa

02776f9

Co-authored-by: Steve Dower <steve.dower@microsoft.com>

Hexi hexa 2

173fa4e

Co-authored-by: Steve Dower <steve.dower@microsoft.com>

terryjreedy reviewed Sep 2, 2022

View reviewed changes

Lib/idlelib/idle_test/test_sidebar.py Show resolved Hide resolved

gpshead merged commit 511ca94 into python:main Sep 2, 2022

bedevere-bot removed the awaiting merge label Sep 2, 2022

gpshead mentioned this pull request Sep 2, 2022

main branch code cleanups for int_max_str_digits non-backportable TODOs #96512

Closed

4 tasks

mdickinson reviewed Sep 3, 2022

View reviewed changes

Doc/library/stdtypes.rst Show resolved Hide resolved

mdickinson reviewed Sep 3, 2022

View reviewed changes

Objects/longobject.c Show resolved Hide resolved

gpshead mentioned this pull request Sep 4, 2022

gh-95778: remove unneeded doc note on float.as_integer_ratio #96553

Merged

light-bringer reviewed Sep 5, 2022

View reviewed changes

mdickinson mentioned this pull request Sep 6, 2022

FR: Change int repr on huge values to automatically use hexadecimal #96601

Closed

rurban reviewed Sep 8, 2022

View reviewed changes

Python/initconfig.c Show resolved Hide resolved

CAM-Gerlach added a commit to CAM-Gerlach/cpython that referenced this pull request Oct 16, 2022

Add int/str security change from issue pythongh-95778 PRs pythongh-96499

12c5273

/ pythongh-95800 Co-authored-by: Gregory P. Smith <gps@google.com>

CAM-Gerlach added a commit to CAM-Gerlach/cpython that referenced this pull request Oct 16, 2022

Add subinterpreters removal from issue pythongh-84694 PRs pythongh-96499

c6d321b

/ pythongh-93306 Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>

CAM-Gerlach added a commit to CAM-Gerlach/cpython that referenced this pull request Oct 16, 2022

Add subinterpreters removal from issue pythongh-84694 PRs pythongh-96499

6c536da

/ pythongh-93306 Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>

CAM-Gerlach mentioned this pull request Oct 16, 2022

gh-95913: Forward-port int/str security change to 3.11 What's New in main #98344

Merged

ucodery mentioned this pull request Dec 16, 2022

Correct CVE-2020-10735 documentation #100306

Merged

itsankitkp mentioned this pull request Jan 5, 2024

sys.flags.int_max_str_digits is not -1 RustPython/RustPython#5139

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-95778: CVE-2020-10735: Prevent DoS by very large int() #96499

gh-95778: CVE-2020-10735: Prevent DoS by very large int() #96499

gpshead commented Sep 2, 2022 •

edited

tiran commented Sep 2, 2022

zooba left a comment

gpshead commented Sep 2, 2022

light-bringer left a comment

septatrix commented Sep 12, 2022

gpshead commented Sep 13, 2022

ngie-eign commented Oct 11, 2022

gh-95778: CVE-2020-10735: Prevent DoS by very large int() #96499

gh-95778: CVE-2020-10735: Prevent DoS by very large int() #96499

Conversation

gpshead commented Sep 2, 2022 • edited

Further Discussion

remaining TODOs (aka project management)

tiran commented Sep 2, 2022

zooba left a comment

Choose a reason for hiding this comment

gpshead commented Sep 2, 2022

light-bringer left a comment

Choose a reason for hiding this comment

septatrix commented Sep 12, 2022

gpshead commented Sep 13, 2022

ngie-eign commented Oct 11, 2022

gpshead commented Sep 2, 2022 •

edited