From 39d683ffeb109872debcb31d39a9b4f671d0fc01 Mon Sep 17 00:00:00 2001 From: clement rouault Date: Mon, 19 Apr 2021 09:23:37 +0200 Subject: [PATCH] Fix & Test infinite look in regkey.values in certains conditions --- tests/test_registry.py | 61 +++++++++++++++++++++++++++++++++++ windows/winobject/registry.py | 23 ++++++++++--- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/tests/test_registry.py b/tests/test_registry.py index 42196a5..0d7ad53 100644 --- a/tests/test_registry.py +++ b/tests/test_registry.py @@ -235,6 +235,67 @@ def test_registry_unicode_subkeys_enumerate(): assert name1 in subkey_names assert name2 in subkey_names +original_RegEnumValueW = windows.winproxy.RegEnumValueW +def fake_RegEnumValueW_fill_but_raise(hKey, dwIndex, lpValueName, lpcchValueName, lpReserved, lpType, lpData, lpcbData): + print("fake_RegEnumValueW") + result = original_RegEnumValueW(hKey, dwIndex, lpValueName, lpcchValueName, lpReserved, lpType, lpData, lpcbData) + raise windows.winproxy.WinproxyError("fake_RegEnumValueW", gdef.ERROR_MORE_DATA) + return result +def test_registry_win_bug_RegEnumValueW_1(monkeypatch): + # import pdb; pdb.set_trace() + # Found a bug on some computers where RegEnumValueW would fill the data but also ERROR_MORE_DATA + basekeytest["VALUE_1"] = "LOOOL" + basekeytest["VALUE_2"] = 42 + basekeytest["XXX" * 0x100] = 42 + assert set(x.name for x in basekeytest.values) == {"VALUE_1", "VALUE_2", "XXX" * 0x100} + monkeypatch.setattr(windows.winproxy, "RegEnumValueW", fake_RegEnumValueW_fill_but_raise) + # Bug make it hang here.. + assert set(x.name for x in basekeytest.values) == {"VALUE_1", "VALUE_2", "XXX" * 0x100} + print("LOL") +def fake_RegEnumValueW_always_raise(hKey, dwIndex, lpValueName, lpcchValueName, lpReserved, lpType, lpData, lpcbData): + print("fake_RegEnumValueW_always_raise") + raise windows.winproxy.WinproxyError("fake_RegEnumValueW", gdef.ERROR_MORE_DATA) + +def test_registry_win_bug_RegEnumValueW_2(monkeypatch): + # Found a bug on some computers where RegEnumValueW would fill the data but also ERROR_MORE_DATA + # This case here should never happen, but better an exception that an infinite loop + basekeytest["VALUE_1"] = "LOOOL" + basekeytest["VALUE_2"] = 42 + basekeytest["XXX" * 0x100] = 42 + assert set(x.name for x in basekeytest.values) == {"VALUE_1", "VALUE_2", "XXX" * 0x100} + monkeypatch.setattr(windows.winproxy, "RegEnumValueW", fake_RegEnumValueW_always_raise) + # Bug make it hang here.. + with pytest.raises(ValueError): + # A bug that do not allow is to extract the values will raises to be explicit.. + assert set(x.name for x in basekeytest.values) == {"VALUE_1", "VALUE_2", "XXX" * 0x100} + +original_basekeytest_get_key_size = basekeytest.get_key_size_info + +def bad_get_key_valuesize(): + namesize, valuesize = original_basekeytest_get_key_size() + return namesize, valuesize - 3 + +def bad_get_key_namesize(): + namesize, valuesize = original_basekeytest_get_key_size() + return namesize - 3, valuesize + +def test_registry_win_bug_get_key_size_info_valuesize_too_small(monkeypatch): + basekeytest["VALUE_1"] = "LOOOL" + basekeytest["VALUE_2"] = 42 + basekeytest["XXX" * 0x100] = 42 + + assert set(x.name for x in basekeytest.values) == {"VALUE_1", "VALUE_2", "XXX" * 0x100} + monkeypatch.setattr(basekeytest, "get_key_size_info", bad_get_key_valuesize) + assert set(x.name for x in basekeytest.values) == {"VALUE_1", "VALUE_2", "XXX" * 0x100} + +def test_registry_win_bug_get_key_size_info_namesize_too_small(monkeypatch): + basekeytest["VALUE_1"] = "LOOOL" + basekeytest["VALUE_2"] = 42 + basekeytest["XXX" * 0x100] = 42 + + assert set(x.name for x in basekeytest.values) == {"VALUE_1", "VALUE_2", "XXX" * 0x100} + monkeypatch.setattr(basekeytest, "get_key_size_info", bad_get_key_namesize) + assert set(x.name for x in basekeytest.values) == {"VALUE_1", "VALUE_2", "XXX" * 0x100} diff --git a/windows/winobject/registry.py b/windows/winobject/registry.py index 39edc15..478192d 100644 --- a/windows/winobject/registry.py +++ b/windows/winobject/registry.py @@ -158,7 +158,7 @@ class PyHKey(object): def _open_key(self, handle, name, sam): result = WinRegistryKey() - winproxy.RegOpenKeyExW(handle, name, 0, sam, result) + winproxy.RegOpenKeyExW(handle, name, 0, sam, result) # TODO: options REG_OPTION_OPEN_LINK dbgprint(u"Opening registry key <{0}> (handle={1:#x})".format(name, result.value), "REGISTRY") return result @@ -241,21 +241,36 @@ class PyHKey(object): databuffer = windows.utils.BUFFER(gdef.BYTE, nbelt=datasize.value)() # A value can have been added in-between. # So recheck the size given by get_key_size_info :) - while True: + # But check 10 times max as RegEnumValueW may bug (seen) and always return ERROR_MORE_DATA even with enought size + for _ in range(10): try: winproxy.RegEnumValueW(self.phkey, i, keyname, namesize, None, value_type, databuffer, datasize) break except WindowsError as e: if e.winerror != gdef.ERROR_MORE_DATA: raise + # I found some strange Windows where even with a big enought buffer: + # - the data was filled + # - ERROR_MORE_DATA was returned + ## To prevent such bug to trigger and infinite loop, two things + # - If the retuned namesize <= the passed keysize and keyname is not empty -> return the data` + # - Max 10 test to prevent Infinite loop + if ((namesize.value <= max_name_size) and (datasize.value <= max_data_size) and + (keyname[:namesize.value].count("\x00") < namesize.value)): # Not just 0 Zero ? + break + # Update the sizes / buffers & try again :) max_name_size, max_data_size = self.get_key_size_info() - max_name_size += 1 - max_data_size += 2 + max_name_size = max(max_name_size + 1, namesize.value + 1) # namesize.value may be > to max_name_size apparently (guessed) + max_data_size = max(max_data_size + 2, datasize.value + 2) # datasize.value may be > to max_data_size apparently (seen) namesize = gdef.DWORD(max_name_size) keyname = ctypes.create_unicode_buffer(namesize.value) datasize = gdef.DWORD(max_data_size) databuffer = windows.utils.BUFFER(gdef.BYTE, nbelt=datasize.value)() + else: + # Probably a windows bug that prevent us from retrieving the data + # Raise something (thus preventing getting the other values..) ? ignore it ? + raise ValueError("Could not extract registry key values, problably a Windows/hook bug") vobj = decode_registry_buffer(value_type.value, databuffer, datasize.value) res.append(KeyValue(keyname.value, vobj, value_type.value)) return res