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

Lib/test/test_winreg.py

@@ -113,7 +113,6 @@ def _write_test_data(self, root_key, subkeystr="sub_key",
  "does not close the actual key!")
  except OSError:
  pass
-
  def _read_test_data(self, root_key, subkeystr="sub_key", OpenKey=OpenKey):
  # Check we can get default value for this key.
  val = QueryValue(root_key, test_key_name)
@@ -340,6 +339,23 @@ def test_setvalueex_value_range(self):
  finally:
  DeleteKey(HKEY_CURRENT_USER, test_key_name)
 
+ 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_dword", None, REG_DWORD, -1)
+ SetValueEx(ck, "test_name_qword", None, REG_QWORD, -1)
+ self.assertRaises(FileNotFoundError, QueryValueEx, ck, "test_name_dword")
+ self.assertRaises(FileNotFoundError, QueryValueEx, ck, "test_name_qword")
+
+ finally:
+ DeleteKey(HKEY_CURRENT_USER, test_key_name)
+
  def test_queryvalueex_return_value(self):
  # Test for Issue #16759, return unsigned int from QueryValueEx.
  # Reg2Py, which gets called by QueryValueEx, was returning a value

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

@@ -0,0 +1,3 @@
+:meth:`winreg.SetValueEx` now leaves the target value untouched in the case of conversion errors.
+Previously, ``-1`` would be written in case of such errors.
+

PC/winreg.c

@@ -561,42 +561,54 @@ Py2Reg(PyObject *value, DWORD typ, BYTE **retDataBuf, DWORD *retDataSize)
 {
  Py_ssize_t i,j;
  switch (typ) {
- case REG_DWORD:
- if (value != Py_None && !PyLong_Check(value))
- return FALSE;
- *retDataBuf = (BYTE *)PyMem_NEW(DWORD, 1);
- if (*retDataBuf == NULL){
- PyErr_NoMemory();
- return FALSE;
- }
- *retDataSize = sizeof(DWORD);
- if (value == Py_None) {
- DWORD zero = 0;
- memcpy(*retDataBuf, &zero, sizeof(DWORD));
- }
- else {
- DWORD d = PyLong_AsUnsignedLong(value);
+ case REG_DWORD: 
+ {
+ if (value != Py_None && !PyLong_Check(value)) {
+ return FALSE;
+ }
+ DWORD d;
+ if (value == Py_None) {
+ d = 0;
+ }
+ else if (PyLong_Check(value)) {
+ d = PyLong_AsUnsignedLong(value);
+ if (d == (DWORD)(-1) && PyErr_Occurred()) {
+ return FALSE;
+ }
+ }
+ *retDataBuf = (BYTE *)PyMem_NEW(DWORD, 1);
+ if (*retDataBuf == NULL) {
+ PyErr_NoMemory();
+ return FALSE;
+ }
  memcpy(*retDataBuf, &d, sizeof(DWORD));
+ *retDataSize = sizeof(DWORD);
+ break;
  }
- break;
- case REG_QWORD:
- if (value != Py_None && !PyLong_Check(value))
- return FALSE;
- *retDataBuf = (BYTE *)PyMem_NEW(DWORD64, 1);
- if (*retDataBuf == NULL){
- PyErr_NoMemory();
- return FALSE;
- }
- *retDataSize = sizeof(DWORD64);
- if (value == Py_None) {
- DWORD64 zero = 0;
- memcpy(*retDataBuf, &zero, sizeof(DWORD64));
- }
- else {
- DWORD64 d = PyLong_AsUnsignedLongLong(value);
+ case REG_QWORD: 
+ {
+ if (value != Py_None && !PyLong_Check(value)) {
+ return FALSE;
+ }
+ DWORD64 d;
+ if (value == Py_None) {
+ d = 0;
+ }
+ else if (PyLong_Check(value)) {
+ d = PyLong_AsUnsignedLongLong(value);
+ if (d == (DWORD64)(-1) && PyErr_Occurred()) {
+ return FALSE;
+ }
+ }
+ *retDataBuf = (BYTE *)PyMem_NEW(DWORD64, 1);
+ if (*retDataBuf == NULL) {
+ PyErr_NoMemory();
+ return FALSE;
+ }
  memcpy(*retDataBuf, &d, sizeof(DWORD64));
+ *retDataSize = sizeof(DWORD64);
+ break;
  }
- break;
  case REG_SZ:
  case REG_EXPAND_SZ:
  {