gh-120378: Fix crash caused by integer overflow in `curses` by ZeroIntensity · Pull Request #124555 · python/cpython

ZeroIntensity · 2024-09-25T22:15:31Z

This is actually an upstream problem in curses, and has been reported to them already. This is a nice workaround in the meantime to prevent the segfault.

Issue: Segmentation Fault in _curses #120378

ZeroIntensity · 2024-09-26T00:13:47Z

cc @picnixz

picnixz · 2024-09-26T00:16:39Z

I'll have a look after sleeping a bit. By the way, instead of cc'ing me, you can just outright request a review from me when you know I've been active on the corresponding issue (I'll reject it if I deem myself not enough an expert).

ZeroIntensity · 2024-09-26T00:18:00Z

I'll have a look after sleeping a bit. By the way, instead of cc'ing me, you can just outright request a review from me when you know I've been active on the corresponding issue (I'll reject it if I deem myself not enough an expert).

I was under the impression that requesting a review didn't result in a notification, good to know!

picnixz

Does the resize_term function also suffer from the same issue?

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

ZeroIntensity · 2024-09-29T14:25:33Z

Does the resize_term function also suffer from the same issue?

It does, but it seems that this patch fixes the issue for both of them. I guess I'll test for it too.

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

…python into curses-int-limit

…lBSz_.rst Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

picnixz · 2024-10-02T12:39:30Z

Ok, by the way I know why we export unconditionally the method. Actually, it would be a no-op if the method is not available (see the clinic generated code which does #define _CURSES_RESIZE_TERM_METHODDEF at the very end). So what I thought was wrong is actually fine.

ZeroIntensity · 2024-10-02T13:39:13Z

I think I fixed it by moving that test, anyway. Anything else to do here?

picnixz · 2024-10-02T13:41:00Z

I think I fixed it by moving that test, anyway. Anything else to do here?

No, nothing else to do! (at least AFAICT).

ZeroIntensity · 2024-10-02T13:55:55Z

Now, time to find a core dev to review this. I don't think we have anyone active that's worked much on curses, do we?

picnixz · 2024-10-02T14:14:13Z

No, but Victor has been reviewing my curses (and not my cursed) PRs so I think we can ask him.

vstinner

LGTM

miss-islington-app · 2024-10-02T14:31:27Z

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

…thonGH-124555) This is actually an upstream problem in curses, and has been reported to them already: https://lists.gnu.org/archive/html/bug-ncurses/2024-09/msg00101.html This is a nice workaround in the meantime to prevent the segfault. (cherry picked from commit c2ba931) Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

miss-islington-app · 2024-10-02T14:31:37Z

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

cherry_picker c2ba931318280796a6dcc33d1a5c5c02ad4d035b 3.12

bedevere-app · 2024-10-02T14:31:44Z

GH-124905 is a backport of this pull request to the 3.13 branch.

vstinner · 2024-10-02T14:48:43Z

@ZeroIntensity: If you consider that backporting to 3.12 is worth it, would you mind to try to backport the change manually? The automated backport failed.

vstinner · 2024-10-02T14:49:15Z

PR merged, thank you @ZeroIntensity. I'm not really super happy about this workaround, but it seems like upstream is not responsive on this crash.

ZeroIntensity · 2024-10-02T15:00:53Z

@ZeroIntensity: If you consider that backporting to 3.12 is worth it, would you mind to try to backport the change manually? The automated backport failed.

Yeah, I can do that a little later today. It's odd that it worked for 3.13 but not 3.12, though.

picnixz · 2024-10-02T16:52:41Z

Yeah, I can do that a little later today. It's odd that it worked for 3.13 but not 3.12, though.

This could be a clinic issue (where the generator changed between 3.12 and 3.13 and introduced conflicts).

…es` (pythonGH-124555) This is actually an upstream problem in curses, and has been reported to them already: https://lists.gnu.org/archive/html/bug-ncurses/2024-09/msg00101.html This is a nice workaround in the meantime to prevent the segfault. (cherry picked from commit c2ba931) Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

bedevere-app · 2024-10-02T21:24:40Z

GH-124911 is a backport of this pull request to the 3.12 branch.

vstinner · 2024-10-02T21:40:18Z

it seems like upstream is not responsive on this crash

Oh, the email was sent last week :-) So it's early to say that upstream is not responsive, sorry about that.

…H-124555) (#124911) This is actually an upstream problem in curses, and has been reported to them already: https://lists.gnu.org/archive/html/bug-ncurses/2024-09/msg00101.html This is a nice workaround in the meantime to prevent the segfault. (cherry picked from commit c2ba931) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

…H-124555) (#124905) gh-120378: Fix crash caused by integer overflow in `curses` (GH-124555) This is actually an upstream problem in curses, and has been reported to them already: https://lists.gnu.org/archive/html/bug-ncurses/2024-09/msg00101.html This is a nice workaround in the meantime to prevent the segfault. (cherry picked from commit c2ba931) Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

ZeroIntensity added 2 commits September 25, 2024 18:06

Fix integer limit for _curses.resizeterm()

6978df9

Add NEWS entry.

a2fbe58

bedevere-app Bot added the awaiting review label Sep 25, 2024

bedevere-app Bot mentioned this pull request Sep 25, 2024

Segmentation Fault in _curses #120378

Closed

ZeroIntensity added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Sep 25, 2024

picnixz self-requested a review September 26, 2024 00:16

picnixz reviewed Sep 29, 2024

View reviewed changes

Comment thread Misc/NEWS.d/next/Library/2024-09-25-18-07-51.gh-issue-120378.NlBSz_.rst Outdated

Comment thread Lib/test/test_curses.py Outdated

Apply suggestions from code review

93eec97

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Test for resizeterm() and resize_term()

fc63d6a

picnixz reviewed Sep 29, 2024

View reviewed changes

Comment thread Lib/test/test_curses.py Outdated

Comment thread Lib/test/test_curses.py Outdated

ZeroIntensity and others added 2 commits September 29, 2024 11:38

Update Lib/test/test_curses.py

db82f15

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Fix tests.

a79bd51

picnixz reviewed Sep 29, 2024

View reviewed changes

Comment thread Lib/test/test_curses.py Outdated

Comment thread Modules/_cursesmodule.c

ZeroIntensity and others added 3 commits September 29, 2024 12:00

Update Lib/test/test_curses.py

679161d

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Run clinic for resize_term()

229e575

Merge branch 'curses-int-limit' of https://github.com/ZeroIntensity/c…

eb20a2e

…python into curses-int-limit

picnixz reviewed Sep 29, 2024

View reviewed changes

Comment thread Misc/NEWS.d/next/Library/2024-09-25-18-07-51.gh-issue-120378.NlBSz_.rst Outdated

Update Misc/NEWS.d/next/Library/2024-09-25-18-07-51.gh-issue-120378.N…

b70a042

…lBSz_.rst Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

picnixz self-requested a review October 2, 2024 12:39

picnixz approved these changes Oct 2, 2024

View reviewed changes

bedevere-app Bot added awaiting core review and removed awaiting review labels Oct 2, 2024

ZeroIntensity requested a review from vstinner October 2, 2024 14:15

vstinner approved these changes Oct 2, 2024

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting core review labels Oct 2, 2024

vstinner enabled auto-merge (squash) October 2, 2024 14:22

vstinner merged commit c2ba931 into python:main Oct 2, 2024

bedevere-app Bot removed the awaiting merge label Oct 2, 2024

miss-islington-app Bot assigned vstinner Oct 2, 2024

bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Oct 2, 2024

ZeroIntensity assigned ZeroIntensity and unassigned vstinner Oct 2, 2024

bedevere-app Bot removed the needs backport to 3.12 only security fixes label Oct 2, 2024

ZeroIntensity deleted the curses-int-limit branch October 2, 2024 21:25

ZeroIntensity mentioned this pull request Oct 23, 2025

SEGV due to ncurses allowing sizes to be excessively large #140462

Closed

Uh oh!

Conversation

ZeroIntensity commented Sep 25, 2024 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZeroIntensity commented Sep 26, 2024

Uh oh!

picnixz commented Sep 26, 2024

Uh oh!

ZeroIntensity commented Sep 26, 2024

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ZeroIntensity commented Sep 29, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

picnixz commented Oct 2, 2024

Uh oh!

ZeroIntensity commented Oct 2, 2024

Uh oh!

picnixz commented Oct 2, 2024

Uh oh!

ZeroIntensity commented Oct 2, 2024

Uh oh!

picnixz commented Oct 2, 2024

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington-app Bot commented Oct 2, 2024

Uh oh!

miss-islington-app Bot commented Oct 2, 2024

Uh oh!

bedevere-app Bot commented Oct 2, 2024

Uh oh!

vstinner commented Oct 2, 2024

Uh oh!

vstinner commented Oct 2, 2024

Uh oh!

ZeroIntensity commented Oct 2, 2024

Uh oh!

picnixz commented Oct 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-app Bot commented Oct 2, 2024

Uh oh!

vstinner commented Oct 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ZeroIntensity commented Sep 25, 2024 •

edited by bedevere-app Bot

Loading

picnixz commented Oct 2, 2024 •

edited

Loading