Better spec compliance for Windows Registery

qiling/os/windows/dlls/advapi32.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-# 
+#
 # Cross Platform and Multi Architecture Advanced Binary Emulation Framework
 #
 
@@ -81,12 +81,23 @@ def __RegQueryValue(ql: Qiling, address: int, params, wstring: bool):
         ql.log.debug("Key value not found")
         return ERROR_FILE_NOT_FOUND
 
+    # read how many bytes we are allowed to write into lpData, however this arg is optional
+    if lpcbData:
+        max_size = ql.mem.read_ptr(lpcbData, 4)
+    else:
+        max_size = 0
+
+        # lpcbData may be null only if lpData is also null. if lpData is allocated, but lpcbData is
+        # set to null, it means we have an out buffer without knowing its size
+        if lpData:
+            return ERROR_INVALID_PARAMETER
+
     # set lpData
-    length = ql.os.registry_manager.write_reg_value_into_mem(reg_type, lpData, value, wstring)
+    length = ql.os.registry_manager.write_reg_value_into_mem(reg_type, lpData, value, max_size, wstring)
 
     # set lpcbData
-    max_size = ql.mem.read_ptr(lpcbData, 4)
-    ql.mem.write_ptr(lpcbData, length, 4)
+    if lpcbData:
+        ql.mem.write_ptr(lpcbData, length, 4)
 
     if max_size < length:
         ret = ERROR_MORE_DATA
@@ -134,6 +145,9 @@ def __RegSetValue(ql: Qiling, address: int, params, wstring: bool):
     # this is done so the print_function would print the correct value
     params["hKey"] = s_hKey
 
+    if not lpData:
+        return ERROR_INVALID_PARAMETER
+
     # dwType is expected to be REG_SZ and lpData to point to a null-terminated string
     ql.os.registry_manager.write(s_hKey, lpSubKey, dwType, lpData, cbData, wstring)
 
@@ -147,8 +161,12 @@ def __RegSetValueEx(ql: Qiling, address: int, params, wstring: bool):
     cbData = params["cbData"]
 
     s_hKey = ql.os.handle_manager.get(hKey).obj
+    # this is done so the print_function would print the correct value
     params["hKey"] = s_hKey
 
+    if not lpData:
+        return ERROR_INVALID_PARAMETER
+
     ql.os.registry_manager.write(s_hKey, lpValueName, dwType, lpData, cbData, wstring)
 
     return ERROR_SUCCESS

qiling/os/windows/registry.py

@@ -1,9 +1,11 @@
 #!/usr/bin/env python3
-# 
+#
 # Cross Platform and Multi Architecture Advanced Binary Emulation Framework
 #
 
-import json, os
+import json
+import os
+
 from Registry import Registry
 from typing import Any, MutableMapping, Optional, Tuple, Union
 
@@ -15,11 +17,11 @@
 # Registry Manager reads data from two places
 # 1. config.json
 #       if you want to modify the registry key/value, you can modify config.json
-#       If there is a registry entry in config.json that needs to be read, 
+#       If there is a registry entry in config.json that needs to be read,
 #       Registry Manager will read from config.json first.
 # 2. windows hive files
 
-# Registry Manager will only write registry changes to config.json 
+# Registry Manager will only write registry changes to config.json
 # and will not modify the hive file.
 
 class RegConf:
@@ -208,14 +210,14 @@ def access(self, key: str, name: Optional[str] = None, type: Optional[int] = Non
         self.ql.os.stats.log_reg_access(key, name, type, value)
 
     def create(self, key: str) -> None:
-       self.regconf.create(key)
-       self.reghive.create(key)
+        self.regconf.create(key)
+        self.reghive.create(key)
 
     def delete(self, key: str, subkey: str) -> None:
-       self.regconf.delete(key, subkey)
-       self.reghive.delete(key, subkey)
+        self.regconf.delete(key, subkey)
+        self.reghive.delete(key, subkey)
 
-    def __reg_mem_read(self, data_type: int, data_addr: int, data_size: int, wide: bool) -> Optional[Union[str, bytes, int]]:
+    def __reg_mem_read(self, data_type: int, data_addr: int, data_size: int, wide: bool) -> Union[str, bytes, int]:
         if data_type in (Registry.RegSZ, Registry.RegExpandSZ):
             os_utils = self.ql.os.utils
             read_string = os_utils.read_wstring if wide else os_utils.read_cstring
@@ -232,11 +234,11 @@ def __reg_mem_read(self, data_type: int, data_addr: int, data_size: int, wide: b
             data = bytes(self.ql.mem.read(data_addr, data_size))
 
         else:
-            data = None
+            raise QlErrorNotImplemented(f'registry type {REG_TYPES[data_type]} not implemented')
 
         return data
 
-    def __reg_mem_write(self, data_type: int, data_addr: int, data_val: Union[str, bytes, int], wide: bool) -> Optional[int]:
+    def __reg_mem_write(self, data_type: int, data_addr: int, data_val: Union[str, bytes, int], max_size: int, wide: bool) -> int:
         if data_type in (Registry.RegSZ, Registry.RegExpandSZ):
             assert type(data_val) is str
 
@@ -259,28 +261,22 @@ def __reg_mem_write(self, data_type: int, data_addr: int, data_val: Union[str, b
             data = data_val
 
         else:
-            return None
+            raise QlErrorNotImplemented(f'registry type {REG_TYPES[data_type]} not implemented')
 
-        self.ql.mem.write(data_addr, data)
+        # in case the out buffer is set to null or it is too small, skip data writing
+        if data_addr and max_size >= len(data):
+            self.ql.mem.write(data_addr, data)
 
         return len(data)
 
     def write(self, key: str, subkey: str, reg_type: int, data_addr: int, data_size: int, wide: bool) -> None:
         data = self.__reg_mem_read(reg_type, data_addr, data_size, wide)
 
-        if data is None:
-            raise QlErrorNotImplemented(f'registry type {REG_TYPES[reg_type]} not implemented')
-
         self.regconf.write(key, subkey, reg_type, data)
         self.reghive.write(key, subkey, reg_type, data)
 
-    def write_reg_value_into_mem(self, data_type: int, data_addr: int, data_val: Union[str, bytes, int], wide: bool) -> int:
-        length = self.__reg_mem_write(data_type, data_addr, data_val, wide)
-
-        if length is None:
-            raise QlErrorNotImplemented(f'registry type {REG_TYPES[data_type]} not implemented')
-
-        return length
+    def write_reg_value_into_mem(self, data_type: int, data_addr: int, data_val: Union[str, bytes, int], max_size: int, wide: bool) -> int:
+        return self.__reg_mem_write(data_type, data_addr, data_val, max_size, wide)
 
     def save(self):
         self.regconf.save(self.regdiff)