b2cae20acd
(CVE-2024-52336) Resolves: RHEL-66636 Added sanity checks for API methods parameters, (CVE-2024-52337) Resolves: RHEL-66613 Signed-off-by: Jaroslav Škarvada <jskarvad@redhat.com>
282 lines
10 KiB
Diff
282 lines
10 KiB
Diff
From 83928aaa29ff281734a12f225a3ea9acd0af96bb Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Jaroslav=20=C5=A0karvada?= <jskarvad@redhat.com>
|
|
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 <jskarvad@redhat.com>
|
|
---
|
|
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 @@
|
|
<defaults>
|
|
<allow_any>auth_admin</allow_any>
|
|
<allow_inactive>auth_admin</allow_inactive>
|
|
- <allow_active>yes</allow_active>
|
|
+ <allow_active>auth_admin</allow_active>
|
|
</defaults>
|
|
</action>
|
|
|
|
@@ -103,7 +103,7 @@
|
|
<defaults>
|
|
<allow_any>auth_admin</allow_any>
|
|
<allow_inactive>auth_admin</allow_inactive>
|
|
- <allow_active>yes</allow_active>
|
|
+ <allow_active>auth_admin</allow_active>
|
|
</defaults>
|
|
</action>
|
|
|
|
@@ -113,7 +113,7 @@
|
|
<defaults>
|
|
<allow_any>auth_admin</allow_any>
|
|
<allow_inactive>auth_admin</allow_inactive>
|
|
- <allow_active>yes</allow_active>
|
|
+ <allow_active>auth_admin</allow_active>
|
|
</defaults>
|
|
</action>
|
|
|
|
@@ -123,7 +123,7 @@
|
|
<defaults>
|
|
<allow_any>auth_admin</allow_any>
|
|
<allow_inactive>auth_admin</allow_inactive>
|
|
- <allow_active>yes</allow_active>
|
|
+ <allow_active>auth_admin</allow_active>
|
|
</defaults>
|
|
</action>
|
|
|
|
@@ -223,7 +223,7 @@
|
|
<defaults>
|
|
<allow_any>auth_admin</allow_any>
|
|
<allow_inactive>auth_admin</allow_inactive>
|
|
- <allow_active>yes</allow_active>
|
|
+ <allow_active>auth_admin</allow_active>
|
|
</defaults>
|
|
</action>
|
|
|
|
@@ -253,7 +253,7 @@
|
|
<defaults>
|
|
<allow_any>auth_admin</allow_any>
|
|
<allow_inactive>auth_admin</allow_inactive>
|
|
- <allow_active>yes</allow_active>
|
|
+ <allow_active>auth_admin</allow_active>
|
|
</defaults>
|
|
</action>
|
|
|
|
@@ -263,7 +263,7 @@
|
|
<defaults>
|
|
<allow_any>auth_admin</allow_any>
|
|
<allow_inactive>auth_admin</allow_inactive>
|
|
- <allow_active>yes</allow_active>
|
|
+ <allow_active>auth_admin</allow_active>
|
|
</defaults>
|
|
</action>
|
|
|
|
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
|
|
|