winreg.SetValueEx causes crash if value = None

[dmo2118]

BPO	21151
Nosy	@PCManticore, @zware, @eryksun, @zooba
Files	fix-none-value.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/zware'
closed_at = <Date 2014-07-03.16:04:14.829>
created_at = <Date 2014-04-04.03:17:01.392>
labels = ['extension-modules', 'OS-windows', 'type-crash']
title = 'winreg.SetValueEx causes crash if value = None'
updated_at = <Date 2014-07-03.16:08:31.757>
user = 'https://bugs.python.org/dmo2118'

bugs.python.org fields:

activity = <Date 2014-07-03.16:08:31.757>
actor = 'zach.ware'
assignee = 'zach.ware'
closed = True
closed_date = <Date 2014-07-03.16:04:14.829>
closer = 'python-dev'
components = ['Extension Modules', 'Windows']
creation = <Date 2014-04-04.03:17:01.392>
creator = 'dmo2118'
dependencies = []
files = ['34840']
hgrepos = []
issue_num = 21151
keywords = ['patch']
message_count = 7.0
messages = ['215486', '215490', '216173', '216637', '220579', '222196', '222198']
nosy_count = 8.0
nosy_names = ['jpe', 'stutzbach', 'Claudiu.Popa', 'python-dev', 'zach.ware', 'eryksun', 'steve.dower', 'dmo2118']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue21151'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

[dmo2118]

Here's a small program that crashes Python 3.

import winreg
winreg.SetValueEx(winreg.HKEY_CURRENT_USER, 'Value', 0, 3, None)

I get a 0xC0000374 exception (STATUS_HEAP_CORRUPTION) when trying to run this. Here's a stack dump:

(snip)
ntdll.dll!RtlpLogHeapFailure+0xa4
ntdll.dll! ?? ::FNODOBFM::`string'+0x10c7c
kernel32.dll!HeapFree+0xa
MSVCR100.dll!free+0x1c
python34.dll!PySetValueEx+0xf8
python34.dll!PyCFunction_Call+0x12d
python34.dll!call_function+0x2ab
python34.dll!PyEval_EvalFrameEx+0x2259
python34.dll!PyEval_EvalCodeEx+0x65c
python34.dll!PyEval_EvalCode+0x2e
python34.dll!builtin_exec+0x1b5
python34.dll!PyCFunction_Call+0x12d
python34.dll!call_function+0x2ab
python34.dll!PyEval_EvalFrameEx+0x2259
python34.dll!PyEval_EvalCodeEx+0x65c
python34.dll!function_call+0x15d
python34.dll!PyObject_Call+0x61
python34.dll!ext_do_call+0x2ab
python34.dll!PyEval_EvalFrameEx+0x22fe
python34.dll!PyEval_EvalCodeEx+0x65c
python34.dll!fast_function+0x14d
python34.dll!call_function+0x311
python34.dll!PyEval_EvalFrameEx+0x2259
python34.dll!PyEval_EvalCodeEx+0x65c
python34.dll!PyEval_EvalCode+0x2e
python34.dll!run_mod+0x53
python34.dll!PyRun_StringFlags+0x9c
python34.dll!PyRun_SimpleStringFlags+0x41
python34.dll!run_command+0x55
python34.dll!Py_Main+0x683
pythonw.exe!__tmainCRTStartup+0x166
kernel32.dll!BaseThreadInitThunk+0xd
ntdll.dll!RtlUserThreadStart+0x1d

System is Windows 7 64-bit, with stock x86-64 Python 3.4.0 binaries.

Incidentally, I was feeding the 'None' to winreg.SetValueEx because that is the value that winreg.EnumValue returns for zero-length binary values. This is somewhat unexpected; I'd personally prefer to get b'' in that instance.

[eryksun]

In Py2Reg, the REG_BINARY (3) case sets *retDataSize = 0 when the value is None:

http://hg.python.org/cpython/file/04f714765c13/PC/winreg.c#l766

It doesn't modify *retDataBuf. Then in PySetValueEx, PyMem_DEL is called for the uninitialized address in data:

http://hg.python.org/cpython/file/04f714765c13/PC/winreg.c#l1566

Py2Reg in this case could also set *retDataBuf = NULL. RegSetValueEx allows lpData to be NULL when cbData is 0.

http://msdn.microsoft.com/en-us/library/ms724923

[jpe]

Here's a simple patch with a test. Oddly, the test doesn't fail before the fix is applied when run with rt.bat, but fails when run directly.

[dmo2118]

Patch works on my end.

[PCManticore]

Hi. You have some trailing whitespaces in the test file (run make patchcheck or python ../Tools/scripts/patchcheck.py). Except that, looks good to me.

[python-dev]

New changeset f2e6c33ce3e9 by Zachary Ware in branch '2.7':
Issue bpo-21151: Fixed a segfault in the _winreg module.
http://hg.python.org/cpython/rev/f2e6c33ce3e9

New changeset 0c5a1835af91 by Zachary Ware in branch '3.4':
Issue bpo-21151: Fixed a segfault in the winreg module.
http://hg.python.org/cpython/rev/0c5a1835af91

New changeset 21cfbcacf0d8 by Zachary Ware in branch 'default':
Closes bpo-21151: Merge with 3.4
http://hg.python.org/cpython/rev/21cfbcacf0d8

[zware]

Thanks to Dave for the report, eryksun for the suggestion, and John for the patch!

I'm not sure why the test case doesn't fail on a regular test run on Python 3; it looks like BYTE *data starts out as NULL when Python is not in interactive mode, which makes no sense to me. The test case does crash on unpatched 2.7, though, so it's a legitimate test in my view.