From 83928aaa29ff281734a12f225a3ea9acd0af96bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20=C5=A0karvada?= Date: Mon, 11 Nov 2024 17:02:44 +0100 Subject: [PATCH] CVE-2024-52336 and CVE-2024-52337 fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - tighten polkit policy - API method parameters sanity check - scripts can be executed only from the profile directories Resolves: CVE-2024-52336 Resolves: CVE-2024-52337 Signed-off-by: Jaroslav Škarvada --- com.redhat.tuned.policy | 14 +++++++------- tuned/consts.py | 4 ++++ tuned/daemon/controller.py | 35 ++++++++++++++++++++++++++-------- tuned/plugins/base.py | 12 ++++++++++++ tuned/plugins/plugin_script.py | 4 ++++ tuned/utils/commands.py | 4 ++++ 6 files changed, 58 insertions(+), 15 deletions(-) diff --git a/com.redhat.tuned.policy b/com.redhat.tuned.policy index f5c972a..077fb74 100644 --- a/com.redhat.tuned.policy +++ b/com.redhat.tuned.policy @@ -43,7 +43,7 @@ auth_admin auth_admin - yes + auth_admin @@ -103,7 +103,7 @@ auth_admin auth_admin - yes + auth_admin @@ -113,7 +113,7 @@ auth_admin auth_admin - yes + auth_admin @@ -123,7 +123,7 @@ auth_admin auth_admin - yes + auth_admin @@ -223,7 +223,7 @@ auth_admin auth_admin - yes + auth_admin @@ -253,7 +253,7 @@ auth_admin auth_admin - yes + auth_admin @@ -263,7 +263,7 @@ auth_admin auth_admin - yes + auth_admin diff --git a/tuned/consts.py b/tuned/consts.py index 912225d..4606aee 100644 --- a/tuned/consts.py +++ b/tuned/consts.py @@ -1,4 +1,8 @@ import logging +import string + +NAMES_ALLOWED_CHARS = string.ascii_letters + string.digits + " !@'+-.,/:;_$&*()%<=>?#[]{|}^~" + '"' +NAMES_MAX_LENGTH = 4096 GLOBAL_CONFIG_FILE = "/etc/tuned/tuned-main.conf" ACTIVE_PROFILE_FILE = "/etc/tuned/active_profile" diff --git a/tuned/daemon/controller.py b/tuned/daemon/controller.py index 4f43d54..726e3a2 100644 --- a/tuned/daemon/controller.py +++ b/tuned/daemon/controller.py @@ -189,6 +189,8 @@ class Controller(tuned.exports.interfaces.ExportableInterface): def switch_profile(self, profile_name, caller = None): if caller == "": return (False, "Unauthorized") + if not self._cmd.is_valid_name(profile_name): + return (False, "Invalid profile_name") return self._switch_profile(profile_name, True) @exports.export("", "(bs)") @@ -262,8 +264,8 @@ class Controller(tuned.exports.interfaces.ExportableInterface): @exports.export("s", "(bsss)") def profile_info(self, profile_name, caller = None): - if caller == "": - return tuple(False, "", "", "") + if caller == "" or not self._cmd.is_valid_name(profile_name): + return (False, "", "", "") if profile_name is None or profile_name == "": profile_name = self.active_profile() return tuple(self._daemon.profile_loader.profile_locator.get_profile_attrs(profile_name, [consts.PROFILE_ATTR_SUMMARY, consts.PROFILE_ATTR_DESCRIPTION], [""])) @@ -294,7 +296,7 @@ class Controller(tuned.exports.interfaces.ExportableInterface): dictionary -- {plugin_name: {parameter_name: default_value}} """ if caller == "": - return False + return {} plugins = {} for plugin_class in self._daemon.get_all_plugins(): plugin_name = plugin_class.__module__.split(".")[-1].split("_", 1)[1] @@ -307,8 +309,8 @@ class Controller(tuned.exports.interfaces.ExportableInterface): @exports.export("s","s") def get_plugin_documentation(self, plugin_name, caller = None): """Return docstring of plugin's class""" - if caller == "": - return False + if caller == "" or not self._cmd.is_valid_name(plugin_name): + return "" return self._daemon.get_plugin_documentation(str(plugin_name)) @exports.export("s","a{ss}") @@ -321,8 +323,8 @@ class Controller(tuned.exports.interfaces.ExportableInterface): Return: dictionary -- {parameter_name: hint} """ - if caller == "": - return False + if caller == "" or not self._cmd.is_valid_name(plugin_name): + return {} return self._daemon.get_plugin_hints(str(plugin_name)) @exports.export("s", "b") @@ -335,7 +337,7 @@ class Controller(tuned.exports.interfaces.ExportableInterface): Return: bool -- True on success """ - if caller == "": + if caller == "" or not self._cmd.is_valid_name(path): return False if self._daemon._application and self._daemon._application._unix_socket_exporter: self._daemon._application._unix_socket_exporter.register_signal_path(path) @@ -349,6 +351,10 @@ class Controller(tuned.exports.interfaces.ExportableInterface): def instance_acquire_devices(self, devices, instance_name, caller = None): if caller == "": return (False, "Unauthorized") + if not self._cmd.is_valid_name(devices): + return (False, "Invalid devices") + if not self._cmd.is_valid_name(instance_name): + return (False, "Invalid instance_name") found = False for instance_target in self._daemon._unit_manager.instances: if instance_target.name == instance_name: @@ -399,6 +405,8 @@ class Controller(tuned.exports.interfaces.ExportableInterface): """ if caller == "": return (False, "Unauthorized", []) + if not self._cmd.is_valid_name(plugin_name): + return (False, "Invalid plugin_name", []) if plugin_name != "" and plugin_name not in self.get_all_plugins().keys(): rets = "Plugin '%s' does not exist" % plugin_name log.error(rets) @@ -422,6 +430,8 @@ class Controller(tuned.exports.interfaces.ExportableInterface): """ if caller == "": return (False, "Unauthorized", []) + if not self._cmd.is_valid_name(instance_name): + return (False, "Invalid instance_name", []) for instance in self._daemon._unit_manager.instances: if instance.name == instance_name: return (True, "OK", sorted(list(instance.processed_devices))) @@ -444,6 +454,13 @@ class Controller(tuned.exports.interfaces.ExportableInterface): """ if caller == "": return (False, "Unauthorized") + if not self._cmd.is_valid_name(plugin_name): + return (False, "Invalid plugin_name") + if not self._cmd.is_valid_name(instance_name): + return (False, "Invalid instance_name") + for (key, value) in options.items(): + if not self._cmd.is_valid_name(key) or not self._cmd.is_valid_name(value): + return (False, "Invalid options") plugins = {p.name: p for p in self._daemon._unit_manager.plugins} if not plugin_name in plugins.keys(): rets = "Plugin '%s' not found" % plugin_name @@ -499,6 +516,8 @@ class Controller(tuned.exports.interfaces.ExportableInterface): """ if caller == "": return (False, "Unauthorized") + if not self._cmd.is_valid_name(instance_name): + return (False, "Invalid instance_name") try: instance = [i for i in self._daemon._unit_manager.instances if i.name == instance_name][0] except IndexError: diff --git a/tuned/plugins/base.py b/tuned/plugins/base.py index cd54aea..3c4122f 100644 --- a/tuned/plugins/base.py +++ b/tuned/plugins/base.py @@ -213,6 +213,14 @@ class Plugin(object): def _instance_post_static(self, instance, enabling): pass + def _safe_script_path(self, path): + path = os.path.realpath(path) + profile_paths = self._global_cfg.get_list(consts.CFG_PROFILE_DIRS, consts.CFG_DEF_PROFILE_DIRS) + for p in profile_paths: + if path.startswith(p): + return True + return False + def _call_device_script(self, instance, script, op, devices, rollback = consts.ROLLBACK_SOFT): if script is None: return None @@ -223,6 +231,10 @@ class Plugin(object): log.error("Relative paths cannot be used in script_pre or script_post. " \ + "Use ${i:PROFILE_DIR}.") return False + if not self._safe_script_path(script): + log.error("Paths outside of the profile directories cannot be used in the " \ + + "script_pre or script_post, ignoring script: '%s'" % script) + return False dir_name = os.path.dirname(script) ret = True for dev in devices: diff --git a/tuned/plugins/plugin_script.py b/tuned/plugins/plugin_script.py index ab605e4..5a5700f 100644 --- a/tuned/plugins/plugin_script.py +++ b/tuned/plugins/plugin_script.py @@ -75,6 +75,10 @@ class ScriptPlugin(base.Plugin): for script in scripts: environ = os.environ environ.update(self._variables.get_env()) + if not self._safe_script_path(script): + log.error("Paths outside of the profile directories cannot be used in the script, " \ + + "ignoring script: '%s'." % script) + continue log.info("calling script '%s' with arguments '%s'" % (script, str(arguments))) log.debug("using environment '%s'" % str(list(environ.items()))) try: diff --git a/tuned/utils/commands.py b/tuned/utils/commands.py index a5a13c3..c4f7c93 100644 --- a/tuned/utils/commands.py +++ b/tuned/utils/commands.py @@ -548,3 +548,7 @@ class commands: import string trans = string.maketrans(source_chars, dest_chars) return text.translate(trans) + + # Checks if name contains only valid characters and has valid length or is empty string or None + def is_valid_name(self, name): + return not name or (all(c in consts.NAMES_ALLOWED_CHARS for c in name) and len(name) <= consts.NAMES_MAX_LENGTH) -- 2.47.0