FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-134873: Fix a DOS issue in `posixpath` by Wulian233 · Pull Request #134927 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-134873: Fix a DOS issue in posixpath - #134927

Closed
Wulian233 wants to merge 1 commit into
python:mainfrom
Wulian233:posixpath-dos
ConversationCommits (1)ChecksFiles changed
Closed

gh-134873: Fix a DOS issue in posixpath#134927
Wulian233 wants to merge 1 commit into
python:mainfrom
Wulian233:posixpath-dos

Conversation

Wulian233 commented May 30, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor 
import time
import os
import posixpath
import Lib.posixpath as posixpath_fixed

os.environ['A'] = 'x'
payload = "$A" * 100000

print("Payload size:", len(payload))

start = time.time()
posixpath.expandvars(payload)
print("Original time:", time.time() - start, "seconds")

start = time.time()
posixpath_fixed.expandvars(payload)
print("Fixed time:", time.time() - start, "seconds")

Above is a code that reproduces the problem, and the speed difference becomes more and more noticeable with the number of times. The above 200,000 times is 15x faster

print:

Payload size: 200000
Original time: 1.7329566478729248 seconds
Fixed time: 0.11450839042663574 seconds

bedevere-app Bot mentioned this pull request May 30, 2025
Closed
ZeroIntensity added type-security A security issue needs backport to 3.9 needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes labels May 30, 2025

serhiy-storchaka commented May 30, 2025

Copy link
Copy Markdown
Member

There is other quadratic complexity here -- in the regular expression. Example: expandvars('${'*1000000).

Also, using re.sub() is more efficient here. See #134952.

Wulian233 commented May 30, 2025

Copy link
Copy Markdown
Contributor Author

Also, using re.sub() is more efficient here. See #134952

#134952 include this fix. What should I do? Should I modify this PR according to your changes or close this PR👀

serhiy-storchaka commented May 31, 2025

Copy link
Copy Markdown
Member

I suggest to close this PR in favor of #134952, unless the latter has serious flaws. Thank you anyway.

Wulian233 commented May 31, 2025

Copy link
Copy Markdown
Contributor Author

No problem

Wulian233 closed this May 31, 2025
Wulian233 deleted the posixpath-dos branch May 31, 2025 07:57
AA-Turner removed needs backport to 3.9 needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes labels Aug 14, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

awaiting review type-security A security issue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL