Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[3.11] gh-95778: Correctly pre-check for int-to-str conversion (GH-96537)#96562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@miss-islington
Copy link
Contributor

@miss-islingtonmiss-islington commentedSep 4, 2022
edited
Loading

Converting a large enoughint to a decimal string raisesValueError as expected. However, the raise comesafter the quadratic-time base-conversion algorithm has run to completion. For effective DOS prevention, we need some kind of check before entering the quadratic-time loop. Oops! =)

The quick fix: essentially we catchmost values that exceed the threshold up front. Those that slip through will still be on the small side (read: sufficiently fast), and will get caught by the existing check so that the limit remains exact.

The justification for the current check. The C code check is:

max_str_digits / (3*PyLong_SHIFT) <= (size_a-11) /10

In GitHub markdown math-speak, writing$M$ formax_str_digits,$L$ forPyLong_SHIFT and$s$ forsize_a, that check is:
$$\left\lfloor\frac{M}{3L}\right\rfloor \le \left\lfloor\frac{s - 11}{10}\right\rfloor$$

From this it follows that
$$\frac{M}{3L} &lt; \frac{s-1}{10}$$
hence that
$$\frac{L(s-1)}{M} &gt; \frac{10}{3} &gt; \log_2(10).$$
So
$$2^{L(s-1)} &gt; 10^M.$$
But our input integer$a$ satisfies$|a| \ge 2^{L(s-1)}$, so$|a|$ is larger than$10^M$. This shows that we don't accidentally capture anythingbelow the intended limit in the check.

Co-authored-by: Gregory P. Smith [Google LLC]greg@krypto.org
(cherry picked from commitb126196)

Co-authored-by: Mark Dickinsondickinsm@gmail.com

Automerge-Triggered-By: GH:gpshead

…GH-96537)Converting a large enough `int` to a decimal string raises `ValueError` as expected. However, the raise comes _after_ the quadratic-time base-conversion algorithm has run to completion. For effective DOS prevention, we need some kind of check before entering the quadratic-time loop. Oops! =)The quick fix: essentially we catch _most_ values that exceed the threshold up front. Those that slip through will still be on the small side (read: sufficiently fast), and will get caught by the existing check so that the limit remains exact.The justification for the current check. The C code check is:```cmax_str_digits / (3 * PyLong_SHIFT) <= (size_a - 11) / 10```In GitHub markdown math-speak, writing $M$ for `max_str_digits`, $L$ for `PyLong_SHIFT` and $s$ for `size_a`, that check is:$$\left\lfloor\frac{M}{3L}\right\rfloor \le \left\lfloor\frac{s - 11}{10}\right\rfloor$$From this it follows that$$\frac{M}{3L} < \frac{s-1}{10}$$hence that$$\frac{L(s-1)}{M} > \frac{10}{3} > \log_2(10).$$So$$2^{L(s-1)} > 10^M.$$But our input integer $a$ satisfies $|a| \ge 2^{L(s-1)}$, so $|a|$ is larger than $10^M$. This shows that we don't accidentally capture anything _below_ the intended limit in the check.<!-- gh-issue-number:pythongh-95778 -->* Issue:pythongh-95778<!-- /gh-issue-number -->Co-authored-by: Gregory P. Smith [Google LLC] <greg@krypto.org>(cherry picked from commitb126196)Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
@bedevere-botbedevere-bot added awaiting review skip news type-bugAn unexpected behavior, bug, or error type-securityA security issue labelsSep 4, 2022
@miss-islington
Copy link
ContributorAuthor

Status check is done, and it's a failure ❌.

@miss-islingtonmiss-islington merged commit8a776d1 intopython:3.11Sep 4, 2022
@miss-islingtonmiss-islington deleted the backport-b126196-3.11 branchSeptember 4, 2022 16:45
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@gpsheadgpsheadgpshead approved these changes

Assignees

No one assigned

Labels

release-blockerskip newstype-bugAn unexpected behavior, bug, or errortype-securityA security issue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@miss-islington@gpshead@bedevere-bot@mdickinson

[8]ページ先頭

©2009-2025 Movatter.jp