Let GefSetting write hooks see value (#1000)

This change makes it so that write hooks see the actual value provided
to a setting when `gef config` is used, and gives it the chance to raise
an exception if the value is invalid.

It also adds a validator 'no_spaces' and adds it to a few settings that
use filepaths, since we know (from #999) that some GDB commands
completely break when paths have spaces and/or when paths are quotes.

gef.py

@@ -9489,9 +9489,11 @@ def __init__(self) -> None:
         gef.config["gef.readline_compat"] = GefSetting(False, bool, "Workaround for readline SOH/ETX issue (SEGV)")
         gef.config["gef.debug"] = GefSetting(False, bool, "Enable debug mode for gef")
         gef.config["gef.autosave_breakpoints_file"] = GefSetting("", str, "Automatically save and restore breakpoints")
-        gef.config["gef.extra_plugins_dir"] = GefSetting("", str, "Autoload additional GEF commands from external directory", hooks={"on_write": self.load_extra_plugins})
+        plugins_dir = GefSetting("", str, "Autoload additional GEF commands from external directory", hooks={"on_write": GefSetting.no_spaces})
+        plugins_dir.add_hook("on_write", lambda _: self.load_extra_plugins())
+        gef.config["gef.extra_plugins_dir"] = plugins_dir
         gef.config["gef.disable_color"] = GefSetting(False, bool, "Disable all colors in GEF")
-        gef.config["gef.tempdir"] = GefSetting(GEF_TEMP_DIR, str, "Directory to use for temporary/cache content")
+        gef.config["gef.tempdir"] = GefSetting(GEF_TEMP_DIR, str, "Directory to use for temporary/cache content", hooks={"on_write": GefSetting.no_spaces})
         gef.config["gef.show_deprecation_warnings"] = GefSetting(True, bool, "Toggle the display of the `deprecated` warnings")
         gef.config["gef.buffer"] = GefSetting(True, bool, "Internally buffer command output until completion")
         gef.config["gef.bruteforce_main_arena"] = GefSetting(False, bool, "Allow bruteforcing main_arena symbol if everything else fails")
@@ -9807,15 +9809,23 @@ def set_setting(self, argv: Tuple[str, Any]) -> None:
         _type = gef.config.raw_entry(key).type
         try:
             if _type == bool:
-                _newval = True if new_value.upper() in ("TRUE", "T", "1") else False
+                if new_value.upper() in ("TRUE", "T", "1"):
+                    _newval = True
+                elif new_value.upper() in ("FALSE", "F", "0"):
+                    _newval = False
+                else:
+                    raise ValueError(f"cannot parse '{new_value}' as bool")
             else:
                 _newval = new_value
-
-            gef.config[key] = _newval
         except Exception as e:
             err(f"'{key}' expects type '{_type.__name__}', got {type(new_value).__name__}: reason {str(e)}")
             return
 
+        try:
+            gef.config[key] = _newval
+        except Exception as e:
+            err(f"Cannot set '{key}': {e}")
+
         reset_all_caches()
         return
 
@@ -10605,30 +10615,34 @@ def malloc_align_address(self, address: int) -> int:
 
 class GefSetting:
     """Basic class for storing gef settings as objects"""
-    READ_ACCESS = 0
-    WRITE_ACCESS = 1
 
     def __init__(self, value: Any, cls: Optional[type] = None, description: Optional[str] = None, hooks: Optional[Dict[str, Callable]] = None)  -> None:
         self.value = value
         self.type = cls or type(value)
         self.description = description or ""
-        self.hooks: Tuple[List[Callable], List[Callable]] = ([], [])
-        if hooks:
-            for access, func in hooks.items():
-                if access == "on_read":
-                    idx = GefSetting.READ_ACCESS
-                elif access == "on_write":
-                    idx = GefSetting.WRITE_ACCESS
-                else:
-                    raise ValueError
-                if not callable(func):
-                    raise ValueError(f"hook is not callable")
-                self.hooks[idx].append(func)
+        self.hooks: Dict[str, List[Callable]] = collections.defaultdict(list)
+        if not hooks:
+            hooks = {}
+
+        for access, func in hooks.items():
+            self.add_hook(access, func)
         return
 
     def __str__(self) -> str:
-        return f"Setting(type={self.type.__name__}, value='{self.value}', desc='{self.description[:10]}...', "\
-            f"read_hooks={len(self.hooks[GefSetting.READ_ACCESS])}, write_hooks={len(self.hooks[GefSetting.READ_ACCESS])})"
+        return f"Setting(type={self.type.__name__}, value='{self.value}', desc='{self.description[:10]}...', " \
+                f"read_hooks={len(self.hooks['on_read'])}, write_hooks={len(self.hooks['on_write'])})"
+
+    def add_hook(self, access, func):
+        if access != "on_read" and access != "on_write":
+            raise ValueError("invalid access type")
+        if not callable(func):
+            raise ValueError("hook is not callable")
+        self.hooks[access].append(func)
+
+    @staticmethod
+    def no_spaces(value):
+        if " " in value:
+            raise ValueError("setting cannot contain spaces")
 
 
 class GefSettingsManager(dict):
@@ -10654,32 +10668,25 @@ def __setitem__(self, name: str, value: Any) -> None:
             if not value.type: raise Exception("Invalid type")
             if not value.description: raise Exception("Invalid description")
             setting = value
+            value = setting.value
         super().__setitem__(name, setting)
-        self.__invoke_write_hooks(setting)
+        self.__invoke_write_hooks(setting, value)
         return
 
     def __delitem__(self, name: str) -> None:
-        super().__delitem__(name)
-        return
+        return super().__delitem__(name)
 
     def raw_entry(self, name: str) -> GefSetting:
         return super().__getitem__(name)
 
     def __invoke_read_hooks(self, setting: GefSetting) -> None:
-        self.__invoke_hooks(is_write=False, setting=setting)
-        return
-
-    def __invoke_write_hooks(self, setting: GefSetting) -> None:
-        self.__invoke_hooks(is_write=True, setting=setting)
+        for callback in setting.hooks["on_read"]:
+            callback()
         return
 
-    def __invoke_hooks(self, is_write: bool, setting: GefSetting) -> None:
-        if not setting.hooks:
-            return
-        idx = int(is_write)
-        if setting.hooks[idx]:
-            for callback in setting.hooks[idx]:
-                callback()
+    def __invoke_write_hooks(self, setting: GefSetting, value: Any) -> None:
+        for callback in setting.hooks["on_write"]:
+            callback(value)
         return
 
 

tests/config/__init__.py

@@ -2,21 +2,50 @@
 Test GEF configuration parameters.
 """
 
-
 from tests.utils import gdb_run_cmd
 from tests.utils import GefUnitTestGeneric
 
 
 class TestGefConfigUnit(GefUnitTestGeneric):
-    """Test GEF configuration paramaters."""
+    """Test GEF configuration parameters."""
 
 
     def test_config_show_opcodes_size(self):
-        """Check opcodes are correctly shown"""
+        """Check opcodes are correctly shown."""
         res = gdb_run_cmd("entry-break", before=["gef config context.show_opcodes_size 4"])
         self.assertNoException(res)
         self.assertGreater(len(res.splitlines()), 1)
 
         # output format: 0xaddress   opcode  <symbol+offset>   mnemo  [operands, ...]
         # example: 0x5555555546b2 897dec      <main+8>         mov    DWORD PTR [rbp-0x14], edi
         self.assertRegex(res, r"(0x([0-9a-f]{2})+)\s+(([0-9a-f]{2})+)\s+<[^>]+>\s+(.*)")
+
+    def test_config_hook_validator(self):
+        """Check that a GefSetting hook can prevent setting a config."""
+        res = gdb_run_cmd("gef config gef.tempdir '/tmp/path with space'")
+        # Validators just use `err` to print an error
+        self.assertNoException(res)
+        self.assertRegex(res, r"[!].+Cannot set.+setting cannot contain spaces")
+
+        res = gdb_run_cmd("gef config gef.tempdir '/tmp/valid-path'")
+        self.assertNoException(res)
+        self.assertNotIn("[!]", res)
+
+    def test_config_type_validator(self):
+        """Check that a GefSetting type can prevent setting a config."""
+        res = gdb_run_cmd("gef config gef.debug invalid")
+        self.assertNoException(res)
+        self.assertRegex(res, r"[!].+expects type 'bool'")
+
+        res = gdb_run_cmd("gef config gef.debug true")
+        self.assertNoException(res)
+        self.assertNotIn("[!]", res)
+        res = gdb_run_cmd("gef config gef.debug 1")
+        self.assertNoException(res)
+        self.assertNotIn("[!]", res)
+        res = gdb_run_cmd("gef config gef.debug F")
+        self.assertNoException(res)
+        self.assertNotIn("[!]", res)
+        res = gdb_run_cmd("gef config gef.debug 0")
+        self.assertNoException(res)
+        self.assertNotIn("[!]", res)