gh-126238: Fix possible null pointer dereference of freevars in _PyCompile_LookupArg #126239

federicovalenso · 2024-10-31T13:12:05Z

Issue: Possible null pointer dereference of freevars in _PyCompile_LookupArg #126238

bedevere-app · 2024-10-31T13:12:09Z

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Misc/NEWS.d/next/Security/2024-10-31-13-14-27.gh-issue-126238.CZqaon.rst

iritkatriel

Can we cover this with a test?

ZeroIntensity · 2024-10-31T13:32:18Z

I'm not exactly sure how to trigger it, it was found via a static analyzer.

…CZqaon.rst Co-authored-by: Peter Bierma <[email protected]>

sobolevn · 2024-10-31T13:37:07Z

Misc/NEWS.d/next/Security/2024-10-31-13-14-27.gh-issue-126238.CZqaon.rst

@@ -0,0 +1 @@
+Fix a possible crash internally when compiling.


I suggest to drop this NEWS entry.

I concur (that's what I did for #126241 because it's a bit hard to phrase it properly and meaningfully).

drop this NEWS entry

Done )

Why? This seems user-facing to me, isn't it?

@sobolevn, @picnixz , what's the bottom line? Is news entry required or not?

The bottom line in general is whether this kind of bug can be triggered easily using public interface. Unless you have a reproducer saying that with X and Y you can do Z, or unless the interface is publicly documented and known to the outside world, a NEWS entry would be fine. But here, we have neither a test nor is _PyCompile_LookupArg something that is exposed to the world.

As an end-user, reading "Fix a possible crash internally when compiling." gives me no information at all except that there was a bug I wasn't aware of (and that it was not always triggerable). I don't know how to make the crash happen, nor do I know what was crashing.

iritkatriel · 2024-10-31T15:54:38Z

Python/compile.c

@@ -901,7 +901,7 @@ _PyCompile_LookupArg(compiler *c, PyCodeObject *co, PyObject *name)
            c->u->u_metadata.u_name,
            co->co_name,
            freevars);


Does PyErr_Format work for null?

Yes, %R formats it as "<NULL>"

ruidazeng · 2024-11-05T16:11:49Z

I'm not exactly sure how to trigger it, it was found via a static analyzer.

Can you be more specific? I cannot reproduce this internal crash.

ZeroIntensity · 2024-11-05T16:21:37Z

Can you be more specific? I cannot reproduce this internal crash.

See GH-126238. Looking at the source, this can only fail in some rare cases when a memory allocation fails, but it's pretty clearly wrong if that was the case so we might as well fix it.

ZeroIntensity

I'm happy with it.

sobolevn

Thank you, congrats on your first merged PR :)

miss-islington-app · 2024-11-05T17:34:37Z

Thanks @federicovalenso for the PR, and @sobolevn for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington-app · 2024-11-05T17:34:41Z

Sorry, @federicovalenso and @sobolevn, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 8525c9375f25e6ec0c0b5dfcab464703f6e78082 3.13

miss-islington-app · 2024-11-05T17:34:45Z

Sorry, @federicovalenso and @sobolevn, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 8525c9375f25e6ec0c0b5dfcab464703f6e78082 3.12

sobolevn · 2024-11-05T17:38:24Z

@federicovalenso do you want to create manual backports? See:

gh-126238: Fix possible null pointer dereference of freevars in _PyCompile_LookupArg #126239 (comment)
gh-126238: Fix possible null pointer dereference of freevars in _PyCompile_LookupArg #126239 (comment)

Or I can do that if you ask me to :)

ZeroIntensity · 2024-11-05T17:41:06Z

See the devguide if you want to know how to do that.

federicovalenso · 2024-11-06T06:40:42Z

@sobolevn , @ZeroIntensity , I'll try to do that)

…vars in _PyCompile_LookupArg (pythonGH-126239) * Replace Py_DECREF by Py_XDECREF (cherry picked from commit 8525c93) Co-authored-by: Valery Fedorenko <[email protected]> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <[email protected]>

federicovalenso · 2024-11-06T07:36:45Z

@sobolevn , could you please look at #126474, #126475 )

…vars in _PyCompile_LookupArg (pythonGH-126239) * Replace Py_DECREF by Py_XDECREF (cherry picked from commit 8525c93) Co-authored-by: Valery Fedorenko <[email protected]> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <[email protected]>

…e_LookupArg (gh-126238) (#126474) [3.12] gh-126238: Fix possible null pointer dereference of freevars in _PyCompile_LookupArg (GH-126239) * Replace Py_DECREF by Py_XDECREF (cherry picked from commit 8525c93) Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <[email protected]>

…e_LookupArg (gh-126238) (#126475) [3.13] gh-126238: Fix possible null pointer dereference of freevars in _PyCompile_LookupArg (GH-126239) * Replace Py_DECREF by Py_XDECREF (cherry picked from commit 8525c93) Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <[email protected]>

… _PyCompile_LookupArg (python#126239) * Replace Py_DECREF by Py_XDECREF Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <[email protected]>

Replace Py_DECREF by Py_XDECREF

f8940e4

bedevere-app bot mentioned this pull request Oct 31, 2024

Possible null pointer dereference of freevars in _PyCompile_LookupArg #126238

Closed

📜🤖 Added by blurb_it.

10dcbe0

federicovalenso marked this pull request as ready for review October 31, 2024 13:15

federicovalenso requested review from markshannon and iritkatriel as code owners October 31, 2024 13:15

bedevere-app bot added the awaiting review label Oct 31, 2024

ZeroIntensity reviewed Oct 31, 2024

View reviewed changes

Misc/NEWS.d/next/Security/2024-10-31-13-14-27.gh-issue-126238.CZqaon.rst Outdated Show resolved Hide resolved

iritkatriel reviewed Oct 31, 2024

View reviewed changes

Update Misc/NEWS.d/next/Security/2024-10-31-13-14-27.gh-issue-126238.…

2c8bc0f

…CZqaon.rst Co-authored-by: Peter Bierma <[email protected]>

sobolevn reviewed Oct 31, 2024

View reviewed changes

picnixz added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes skip news labels Oct 31, 2024

iritkatriel reviewed Oct 31, 2024

View reviewed changes

Remove news entry due to "skip news" label

547eca1

ruidazeng approved these changes Nov 5, 2024

View reviewed changes

bedevere-app bot added awaiting core review and removed awaiting review labels Nov 5, 2024

ZeroIntensity approved these changes Nov 5, 2024

View reviewed changes

sobolevn approved these changes Nov 5, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting core review labels Nov 5, 2024

sobolevn merged commit 8525c93 into python:main Nov 5, 2024
45 checks passed

bedevere-app bot removed the awaiting merge label Nov 5, 2024

miss-islington-app bot assigned sobolevn Nov 5, 2024

federicovalenso mentioned this pull request Nov 6, 2024

[3.13] Fix possible null pointer dereference of freevars in _PyCompile_LookupArg (gh-126238) #126475

Merged

ambv removed needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Nov 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-126238: Fix possible null pointer dereference of freevars in _PyCompile_LookupArg #126239

gh-126238: Fix possible null pointer dereference of freevars in _PyCompile_LookupArg #126239

federicovalenso commented Oct 31, 2024 •

edited by bedevere-app bot

Loading

bedevere-app bot commented Oct 31, 2024

iritkatriel left a comment

ZeroIntensity commented Oct 31, 2024

sobolevn Oct 31, 2024

picnixz Oct 31, 2024

federicovalenso Nov 1, 2024

ZeroIntensity Nov 1, 2024

federicovalenso Nov 1, 2024

picnixz Nov 1, 2024 •

edited

Loading

iritkatriel Oct 31, 2024

sobolevn Oct 31, 2024

ruidazeng commented Nov 5, 2024

ZeroIntensity commented Nov 5, 2024

ZeroIntensity left a comment

sobolevn left a comment

miss-islington-app bot commented Nov 5, 2024

miss-islington-app bot commented Nov 5, 2024

miss-islington-app bot commented Nov 5, 2024

sobolevn commented Nov 5, 2024

ZeroIntensity commented Nov 5, 2024

federicovalenso commented Nov 6, 2024

federicovalenso commented Nov 6, 2024 •

edited

Loading

		@@ -0,0 +1 @@
		Fix a possible crash internally when compiling.

gh-126238: Fix possible null pointer dereference of freevars in _PyCompile_LookupArg #126239

gh-126238: Fix possible null pointer dereference of freevars in _PyCompile_LookupArg #126239

Conversation

federicovalenso commented Oct 31, 2024 • edited by bedevere-app bot Loading

bedevere-app bot commented Oct 31, 2024

iritkatriel left a comment

Choose a reason for hiding this comment

ZeroIntensity commented Oct 31, 2024

sobolevn Oct 31, 2024

Choose a reason for hiding this comment

picnixz Oct 31, 2024

Choose a reason for hiding this comment

federicovalenso Nov 1, 2024

Choose a reason for hiding this comment

ZeroIntensity Nov 1, 2024

Choose a reason for hiding this comment

federicovalenso Nov 1, 2024

Choose a reason for hiding this comment

picnixz Nov 1, 2024 • edited Loading

Choose a reason for hiding this comment

iritkatriel Oct 31, 2024

Choose a reason for hiding this comment

sobolevn Oct 31, 2024

Choose a reason for hiding this comment

ruidazeng commented Nov 5, 2024

ZeroIntensity commented Nov 5, 2024

ZeroIntensity left a comment

Choose a reason for hiding this comment

sobolevn left a comment

Choose a reason for hiding this comment

miss-islington-app bot commented Nov 5, 2024

miss-islington-app bot commented Nov 5, 2024

miss-islington-app bot commented Nov 5, 2024

sobolevn commented Nov 5, 2024

ZeroIntensity commented Nov 5, 2024

federicovalenso commented Nov 6, 2024

federicovalenso commented Nov 6, 2024 • edited Loading

federicovalenso commented Oct 31, 2024 •

edited by bedevere-app bot

Loading

picnixz Nov 1, 2024 •

edited

Loading

federicovalenso commented Nov 6, 2024 •

edited

Loading