Skip to content
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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

gh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing #99890

Merged
merged 9 commits into from
Dec 17, 2022
Merged

gh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing #99890

merged 9 commits into from
Dec 17, 2022

Conversation

colorfulappl
Copy link
Contributor

@colorfulappl colorfulappl commented Nov 30, 2022 •
edited by bedevere-bot
Loading

In the family of _PyArg_Parse functions, pointers can be passed in as arguments to store the parsed PyArgs.
If a pointer is assigned to newly allocated memory and the memory is freed as a result of an error while parsing the subsequent PyArgs, the pointer should be set to NULL; otherwise, it leaks the freed memory.

@bedevere-bot bedevere-bot mentioned this pull request Nov 30, 2022
Closed
@colorfulappl
Copy link
Contributor Author

colorfulappl commented Nov 30, 2022 •
edited
Loading

This fix corresponds to my comment in #99240

If function _PyArg_ParseStack parses failed, assign all the parsed arguments to "NULL" after they are freed, this should be done in _PyArg_ParseStack.

kumaraditya303
kumaraditya303 reviewed Dec 13, 2022
View reviewed changes
Python/getargs.c Outdated Show resolved Hide resolved
@python python deleted a comment from netlify bot Dec 14, 2022
@kumaraditya303
Copy link
Contributor

Can you add a test for this?

@carljm carljm changed the title gh-99240: Reset pointer to NULL then the pointed memory is freed in argument parsing gh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing Dec 15, 2022
@kumaraditya303 kumaraditya303 self-assigned this Dec 16, 2022
kumaraditya303
kumaraditya303 approved these changes Dec 16, 2022
View reviewed changes
Copy link
Contributor

@kumaraditya303 kumaraditya303 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kumaraditya303 kumaraditya303 added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Dec 16, 2022
erlend-aasland
erlend-aasland approved these changes Dec 16, 2022
View reviewed changes
Modules/_testcapi/getargs.c Outdated Show resolved Hide resolved
@kumaraditya303 kumaraditya303 merged commit a6f82f1 into python:main Dec 17, 2022
@miss-islington
Copy link
Contributor

Thanks @colorfulappl for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @colorfulappl and @kumaraditya303, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker a6f82f1fc68cb24e2d88d35fde4cfb663213a744 3.11

@miss-islington
Copy link
Contributor

Sorry, @colorfulappl and @kumaraditya303, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker a6f82f1fc68cb24e2d88d35fde4cfb663213a744 3.10

@kumaraditya303
Copy link
Contributor

@colorfulappl Can you create the backport PRs? Thanks

kumaraditya303 added a commit to kumaraditya303/cpython that referenced this pull request Dec 17, 2022
@colorfulappl @kumaraditya303 @erlend-aasland
pythongh-99240: Reset pointer to NULL when the pointed memory is free…
efbb1eb 
…d in argument parsing (python#99890)

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
shihai1991 added a commit to shihai1991/cpython that referenced this pull request Dec 18, 2022
@shihai1991
Merge remote-tracking branch 'origin/main' into bpo_34697
2878571 
* origin/main: (1306 commits)
  Correct CVE-2020-10735 documentation (python#100306)
  pythongh-100272: Fix JSON serialization of OrderedDict (pythonGH-100273)
  pythongh-93649: Split tracemalloc tests from _testcapimodule.c (python#99551)
  Docs: Use `PY_VERSION_HEX` for version comparison (python#100179)
  pythongh-97909: Fix markup for `PyMethodDef` members (python#100089)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythonGH-98831: Add DECREF_INPUTS(), expanding to DECREF() each stack input (python#100205)
  pythongh-78707: deprecate passing >1 argument to `PurePath.[is_]relative_to()` (pythonGH-94469)
  pythongh-99540: Constant hash for _PyNone_Type to aid reproducibility (pythonGH-99541)
  pythongh-100039: enhance __signature__ to work with str and callables (pythonGH-100168)
  pythongh-99830: asyncio: Document returns of remove_{reader,writer} (python#100302)
  "Compound statement" docs: Fix with-statement step indexing (python#100286)
  pythonGH-90043: Handle NaNs in COMPARE_OP_FLOAT_JUMP (pythonGH-100278)
  Improve stats presentation for calls. (pythonGH-100274)
  Better stats for `LOAD_ATTR` and `STORE_ATTR` (pythonGH-100295)
  pythongh-81057: Move the Cached Parser Dummy Name to _PyRuntimeState (python#100277)
  Document that zipfile's pwd parameter is a `bytes` object (python#100209)
  pythongh-99767: mark `PyTypeObject.tp_watched` as internal use only in table (python#100271)
  Fix typo in introduction.rst (python#100266)
  ...
@colorfulappl
Copy link
Contributor Author

@colorfulappl Can you create the backport PRs?

Sure. I will do this later.

carljm added a commit to carljm/cpython that referenced this pull request Dec 19, 2022
@carljm
Merge branch 'main' into fastmock
a9af20a 
* main:
  pythongh-89727: Fix os.walk RecursionError on deep trees (python#99803)
  Docs: Don't upload CI artifacts (python#100330)
  pythongh-94912: Added marker for non-standard coroutine function detection (python#99247)
  Correct CVE-2020-10735 documentation (python#100306)
  pythongh-100272: Fix JSON serialization of OrderedDict (pythonGH-100273)
  pythongh-93649: Split tracemalloc tests from _testcapimodule.c (python#99551)
  Docs: Use `PY_VERSION_HEX` for version comparison (python#100179)
  pythongh-97909: Fix markup for `PyMethodDef` members (python#100089)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythonGH-98831: Add DECREF_INPUTS(), expanding to DECREF() each stack input (python#100205)
  pythongh-78707: deprecate passing >1 argument to `PurePath.[is_]relative_to()` (pythonGH-94469)
colorfulappl added a commit to colorfulappl/cpython that referenced this pull request Dec 21, 2022
@colorfulappl @kumaraditya303 @erlend-aasland
[3.11] pythongh-99240: Reset pointer to NULL when the pointed memory …
62c4a95 
…is freed in argument parsing (pythonGH-99890)

(cherry picked from commit efbb1eb)

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@bedevere-bot
Copy link

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

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Dec 21, 2022
colorfulappl added a commit to colorfulappl/cpython that referenced this pull request Dec 21, 2022
@colorfulappl @kumaraditya303 @erlend-aasland
[3.10] pythongh-99240: Reset pointer to NULL when the pointed memory …
e02b61b 
…is freed in argument parsing (pythonGH-99890)

(cherry picked from commit efbb1eb)

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@bedevere-bot
Copy link

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

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Dec 21, 2022
kumaraditya303 added a commit that referenced this pull request Dec 21, 2022
@colorfulappl @kumaraditya303 @erlend-aasland
[3.11] gh-99240: Reset pointer to NULL when the pointed memory is fre…
bed1d14 
…ed in argument parsing (GH-99890) (#100385)

(cherry picked from commit efbb1eb)

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
kumaraditya303 added a commit that referenced this pull request Dec 21, 2022
@colorfulappl @kumaraditya303 @erlend-aasland
[3.10] gh-99240: Reset pointer to NULL when the pointed memory is fre…
591365c 
…ed in argument parsing (GH-99890) (#100386)

(cherry picked from commit efbb1eb)

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
rwgk pushed a commit to rwgk/cpython that referenced this pull request Mar 11, 2023
@colorfulappl @kumaraditya303 @erlend-aasland
pythongh-99240: Reset pointer to NULL when the pointed memory is free…
9aad32a 
…d in argument parsing (python#99890)

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes

@erlend-aasland erlend-aasland erlend-aasland approved these changes

Assignees

@kumaraditya303 kumaraditya303

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@colorfulappl @kumaraditya303 @miss-islington @bedevere-bot @erlend-aasland