bpo-43984: Update winreg.SetValueEx to make sure the value set is not -1 by shreyanavigyan · Pull Request #25775 · python/cpython

shreyanavigyan · 2021-05-01T09:48:28Z

bpo-43984: Update winreg.SetValueEx to make sure the value set is not -1

Fix REG_QWORD case in Py2Reg
Fix REG_DWORD case in Py2Reg
Add test
Make sure the test does not appear as virus to the antivirus software!

https://bugs.python.org/issue43984

shreyanavigyan · 2021-05-01T09:48:51Z

Skip news

shreyanavigyan · 2021-05-02T09:25:01Z

Can anyone please add a skip news label to this PR?

erlend-aasland · 2021-05-02T09:45:57Z

Can anyone please add a skip news label to this PR?

Isn't this a change of behaviour? If so, I'd say it calls for a NEWS item.

shreyanavigyan · 2021-05-02T10:00:33Z

News entry added

erlend-aasland · 2021-05-02T11:12:27Z

Perhaps you could use the bug report to write a test case? (Lib/test/test_winreg.py)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>

shreyanavigyan · 2021-05-08T19:49:48Z

+ def test_setvalueex_negative_one_check(self):
+ # Test for Issue #43984, check -1 was not set by SetValueEx.
+ # Py2Reg, which gets called by SetValueEx, wasn't checking return
+ # value by PyLong_AsUnsignedLong, thus setting -1 as value in the registry.
+ # The implementation now checks PyLong_AsUnsignedLong return value to assure
+ # the value set was not -1.
+ try:
+ with CreateKey(HKEY_CURRENT_USER, test_key_name) as ck:
+ with self.assertRaises(OverflowError):
+ SetValueEx(ck, "test_name", None, REG_DWORD, -1)
+ with self.assertRaises(FileNotFoundError):
+ with QueryValueEx(ck, "test_name") as subkey:
+ pass
+ finally:
+ DeleteKey(HKEY_CURRENT_USER, test_key_name)


There's something wrong with this test I added. My antivirus is identifying this piece of code as virus. Any thoughts?

If any maintainers figure out the problem please add the required commit. Who cannot add commits please suggest the patch, I'll commit it.

github-actions · 2021-06-10T00:04:29Z

This PR is stale because it has been open for 30 days with no activity.

shreyanavigyan · 2021-06-10T16:14:03Z

A Friendly Ping.

zooba · 2021-07-06T23:37:01Z

Sorry for the lack of review. Unfortunately, it looks like someone added a new required check, so I'll close/reopen to re-run it, but the change looks okay to me. If someone else gets back to this before I do, they can merge.

netlify · 2022-12-08T18:56:06Z

✅ Deploy Preview for python-cpython-preview canceled.

Name	Link
🔨 Latest commit	`06da229`
🔍 Latest deploy log	https://app.netlify.com/sites/python-cpython-preview/deploys/6393215daba0e50008844eb4

zooba · 2022-12-09T11:39:51Z

The test failure on Azure Pipelines is patchcheck reporting extra whitespace in the new test. I can't see it in the diff, but I find it's usually trailing whitespace in a comment.

We shouldn't merge with that failure, or it'll show up randomly in the future.

miss-islington · 2022-12-09T12:47:21Z

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

bedevere-bot · 2022-12-09T12:47:43Z

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

bedevere-bot · 2022-12-09T12:47:48Z

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

zooba · 2022-12-09T12:47:51Z

Thanks! FWIW, I suspect the antimalware detection was probably because it's an unsigned binary messing with the registry. Once we do our signed release, I'm sure it'll be fine.

…n error (pythonGH-25775) (cherry picked from commit a29a7b9) Co-authored-by: Shreyan Avigyan <shreyan.avigyan@gmail.com>

…n error (GH-25775) (cherry picked from commit a29a7b9) Co-authored-by: Shreyan Avigyan <shreyan.avigyan@gmail.com>

Fix for bpo-43984
1f2d523

shreyanavigyan requested a review from a team as a code owner May 1, 2021 09:48

bedevere-bot added the awaiting review label May 1, 2021

the-knights-who-say-ni added the CLA signed label May 1, 2021

shreyanavigyan changed the title ~~bpo-43984: Update winreg.SetValueEx to make sure the returned value is not -1~~bpo-43984: Update winreg.SetValueEx to make sure the value set is not -1May 1, 2021

erlend-aasland reviewed May 2, 2021
View reviewed changes

Comment threadPC/winreg.c

Update - Free retDataBuf before returning FALSE
b00fc84

erlend-aasland reviewed May 2, 2021
View reviewed changes

Comment threadPC/winreg.c Outdated

Update winreg.c
7841714

Add news entry
1123340

shreyanavigyan requested review from pfmoore and removed request for a teamMay 2, 2021 10:55

python deleted a comment May 4, 2021

erlend-aasland reviewed May 8, 2021
View reviewed changes

Comment threadMisc/NEWS.d/next/Windows/2021-05-02-15-29-33.bpo-43984.U92jiv.rst Outdated

shreyanavigyan added 2 commits May 8, 2021 23:59

Move the error check up before allocation
7bbb3f2

Update 2021-05-02-15-29-33.bpo-43984.U92jiv.rst
c5755dd

erlend-aasland requested changes May 8, 2021
View reviewed changes

Comment threadPC/winreg.c Outdated
Comment threadPC/winreg.c Outdated
Comment threadMisc/NEWS.d/next/Windows/2021-05-02-15-29-33.bpo-43984.U92jiv.rst Outdated

bedevere-bot added awaiting core review and removed awaiting review labels May 8, 2021

shreyanavigyanand others added 4 commits May 9, 2021 00:35

Update PC/winreg.c
ad67138
Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>

Update PC/winreg.c
ec0ea5a
Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>

Add test
62541b8

Fix typo
27b8130

shreyanavigyan commented May 8, 2021
View reviewed changes

Update 2021-05-02-15-29-33.bpo-43984.U92jiv.rst
41a70e0

github-actionsBot added the stale Stale PR or inactive for long period of time. label Jun 10, 2021

github-actionsBot removed the stale Stale PR or inactive for long period of time. label Jun 11, 2021

zooba closed this Jul 6, 2021

zooba reopened this Jul 6, 2021

ezio-melotti removed the CLA signed label Jul 13, 2022

Merge branch 'main' into bpo-43984
666742c

AlexWaygood reviewed Dec 9, 2022
View reviewed changes

Comment threadLib/test/test_winreg.py Outdated
Comment threadLib/test/test_winreg.py Outdated

Fix whitespace
afdee9f

AlexWaygood reviewed Dec 9, 2022
View reviewed changes

Comment threadLib/test/test_winreg.py
Comment threadLib/test/test_winreg.py

Never edit on your phone
06da229

zooba added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Dec 9, 2022

zooba merged commit a29a7b9 into python:mainDec 9, 2022

bedevere-bot removed the awaiting core review label Dec 9, 2022

bedevere-bot removed the needs backport to 3.11 only security fixes label Dec 9, 2022

bedevere-bot removed the needs backport to 3.10 only security fixes label Dec 9, 2022

r3pwnxmannequin mentioned this pull request Apr 10, 2022
winreg.SetValueEx should check the returned value #88150
Closed

miss-islington added a commit that referenced this pull request Dec 9, 2022
bpo-43984: Allow winreg.SetValueEx to set -1 without treating it as a…
3939a4b
…n error (GH-25775) (cherry picked from commit a29a7b9) Co-authored-by: Shreyan Avigyan <shreyan.avigyan@gmail.com>

miss-islington added a commit that referenced this pull request Dec 9, 2022
bpo-43984: Allow winreg.SetValueEx to set -1 without treating it as a…
580165d
…n error (GH-25775) (cherry picked from commit a29a7b9) Co-authored-by: Shreyan Avigyan <shreyan.avigyan@gmail.com>

Uh oh!

Uh oh!

Conversation

shreyanavigyan commented May 1, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shreyanavigyan commented May 1, 2021

Uh oh!

Uh oh!

Uh oh!

shreyanavigyan commented May 2, 2021

Uh oh!

erlend-aasland commented May 2, 2021

Uh oh!

shreyanavigyan commented May 2, 2021

Uh oh!

erlend-aasland commented May 2, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shreyanavigyanMay 8, 2021

Choose a reason for hiding this comment

Uh oh!

shreyanavigyanMay 8, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actionsBot commented Jun 10, 2021

Uh oh!

shreyanavigyan commented Jun 10, 2021

Uh oh!

zooba commented Jul 6, 2021

Uh oh!

netlifyBot commented Dec 8, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for python-cpython-preview canceled.

Uh oh!

zooba commented Dec 9, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

miss-islington commented Dec 9, 2022

Uh oh!

bedevere-bot commented Dec 9, 2022

Uh oh!

bedevere-bot commented Dec 9, 2022

Uh oh!

zooba commented Dec 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

shreyanavigyan commented May 1, 2021•
edited
Loading

shreyanavigyanMay 8, 2021•
edited
Loading

netlifyBot commented Dec 8, 2022•
edited
Loading