diff mbox series

[langdale,28/41] fwupd: Fix CVE-2022-3287

Message ID f34c046760d0910ba7bf5538d501ddaf93096462.1674653280.git.akuster808@gmail.com
State New
Headers show
Series [langdale,01/41] blueman: add RDEPEND on python3-fcntl | expand

Commit Message

akuster808 Jan. 25, 2023, 1:31 p.m. UTC
From: Chee Yang Lee <chee.yang.lee@intel.com>

Signed-off-by: Chee Yang Lee <chee.yang.lee@intel.com>
Signed-off-by: Armin Kuster <akuster808@gmail.com>
---
 .../fwupd/fwupd/CVE-2022-3287.patch           | 218 ++++++++++++++++++
 meta-oe/recipes-bsp/fwupd/fwupd_1.8.4.bb      |   4 +-
 2 files changed, 221 insertions(+), 1 deletion(-)
 create mode 100644 meta-oe/recipes-bsp/fwupd/fwupd/CVE-2022-3287.patch
diff mbox series

Patch

diff --git a/meta-oe/recipes-bsp/fwupd/fwupd/CVE-2022-3287.patch b/meta-oe/recipes-bsp/fwupd/fwupd/CVE-2022-3287.patch
new file mode 100644
index 0000000000..5360e981ce
--- /dev/null
+++ b/meta-oe/recipes-bsp/fwupd/fwupd/CVE-2022-3287.patch
@@ -0,0 +1,218 @@ 
+From ea676855f2119e36d433fbd2ed604039f53b2091 Mon Sep 17 00:00:00 2001
+From: Richard Hughes <richard@hughsie.com>
+Date: Wed, 21 Sep 2022 14:56:10 +0100
+Subject: [PATCH] Never save the Redfish passwords to a file readable by users
+
+When the redfish plugin automatically creates an OPERATOR user account on the
+BMC we save the autogenerated password to /etc/fwupd/redfish.conf, ensuring it
+is chmod'ed to 0660 before writing the file with g_key_file_save_to_file().
+
+Under the covers, g_key_file_save_to_file() calls g_file_set_contents() with
+the keyfile string data.
+I was under the impression that G_FILE_CREATE_REPLACE_DESTINATION was being
+used to copy permissions, but alas not.
+
+GLib instead calls g_file_set_contents_full() with the mode hardcoded to 0666,
+which undoes the previous chmod().
+
+Use g_file_set_contents_full() with the correct mode for newer GLib versions,
+and provide a fallback with the same semantics for older versions.
+
+https://github.com/fwupd/fwupd/commit/ea676855f2119e36d433fbd2ed604039f53b2091
+Upstream-Status: Backport
+CVE: CVE-2022-3287
+Signed-off-by: Chee Yang Lee <chee.yang.lee@intel.com>
+
+---
+ contrib/fwupd.spec.in         |  3 ++
+ libfwupdplugin/fu-plugin.c    | 65 +++++++++++++++++++++++++++++------
+ libfwupdplugin/fu-self-test.c | 57 ++++++++++++++++++++++++++++++
+ 3 files changed, 114 insertions(+), 11 deletions(-)
+
+diff --git a/contrib/fwupd.spec.in b/contrib/fwupd.spec.in
+index b011292b1b..42ea2024a8 100644
+--- a/contrib/fwupd.spec.in
++++ b/contrib/fwupd.spec.in
+@@ -326,6 +326,9 @@ for fn in /etc/fwupd/remotes.d/*.conf; do
+     fi
+ done
+ 
++# ensure this is private
++chmod 0660 /etc/fwupd/redfish.conf
++
+ %preun
+ %systemd_preun fwupd.service
+ 
+diff --git a/libfwupdplugin/fu-plugin.c b/libfwupdplugin/fu-plugin.c
+index 9744af9d60..b431f6d418 100644
+--- a/libfwupdplugin/fu-plugin.c
++++ b/libfwupdplugin/fu-plugin.c
+@@ -9,6 +9,7 @@
+ #include "config.h"
+ 
+ #include <errno.h>
++#include <fcntl.h>
+ #include <fwupd.h>
+ #include <glib/gstdio.h>
+ #include <gmodule.h>
+@@ -2417,6 +2418,46 @@ fu_plugin_set_config_value(FuPlugin *self, const gchar *key, const gchar *value,
+ 	return g_key_file_save_to_file(keyfile, conf_path, error);
+ }
+ 
++#if !GLIB_CHECK_VERSION(2, 66, 0)
++
++#define G_FILE_SET_CONTENTS_CONSISTENT 0
++typedef guint GFileSetContentsFlags;
++static gboolean
++g_file_set_contents_full(const gchar *filename,
++			 const gchar *contents,
++			 gssize length,
++			 GFileSetContentsFlags flags,
++			 int mode,
++			 GError **error)
++{
++	gint fd;
++	gssize wrote;
++
++	if (length < 0)
++		length = strlen(contents);
++	fd = g_open(filename, O_CREAT, mode);
++	if (fd <= 0) {
++		g_set_error(error,
++			    G_IO_ERROR,
++			    G_IO_ERROR_FAILED,
++			    "could not open %s file",
++			    filename);
++		return FALSE;
++	}
++	wrote = write(fd, contents, length);
++	if (wrote != length) {
++		g_set_error(error,
++			    G_IO_ERROR,
++			    G_IO_ERROR_FAILED,
++			    "did not write %s file",
++			    filename);
++		g_close(fd, NULL);
++		return FALSE;
++	}
++	return g_close(fd, error);
++}
++#endif
++
+ /**
+  * fu_plugin_set_secure_config_value:
+  * @self: a #FuPlugin
+@@ -2438,7 +2479,8 @@ fu_plugin_set_secure_config_value(FuPlugin *self,
+ 				  GError **error)
+ {
+ 	g_autofree gchar *conf_path = fu_plugin_get_config_filename(self);
+-	gint ret;
++	g_autofree gchar *data = NULL;
++	g_autoptr(GKeyFile) keyfile = g_key_file_new();
+ 
+ 	g_return_val_if_fail(FU_IS_PLUGIN(self), FALSE);
+ 	g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
+@@ -2447,17 +2489,18 @@ fu_plugin_set_secure_config_value(FuPlugin *self,
+ 		g_set_error(error, FWUPD_ERROR, FWUPD_ERROR_NOT_FOUND, "%s is missing", conf_path);
+ 		return FALSE;
+ 	}
+-	ret = g_chmod(conf_path, 0660);
+-	if (ret == -1) {
+-		g_set_error(error,
+-			    FWUPD_ERROR,
+-			    FWUPD_ERROR_INTERNAL,
+-			    "failed to set permissions on %s",
+-			    conf_path);
++	if (!g_key_file_load_from_file(keyfile, conf_path, G_KEY_FILE_KEEP_COMMENTS, error))
+ 		return FALSE;
+-	}
+-
+-	return fu_plugin_set_config_value(self, key, value, error);
++	g_key_file_set_string(keyfile, fu_plugin_get_name(self), key, value);
++	data = g_key_file_to_data(keyfile, NULL, error);
++	if (data == NULL)
++		return FALSE;
++	return g_file_set_contents_full(conf_path,
++					data,
++					-1,
++					G_FILE_SET_CONTENTS_CONSISTENT,
++					0660,
++					error);
+ }
+ 
+ /**
+diff --git a/libfwupdplugin/fu-self-test.c b/libfwupdplugin/fu-self-test.c
+index 2dbc9c94ff..aaf49c172b 100644
+--- a/libfwupdplugin/fu-self-test.c
++++ b/libfwupdplugin/fu-self-test.c
+@@ -674,6 +674,62 @@ _plugin_device_added_cb(FuPlugin *plugin, FuDevice *device, gpointer user_data)
+ 	fu_test_loop_quit();
+ }
+ 
++static void
++fu_plugin_config_func(void)
++{
++	GStatBuf statbuf = {0};
++	gboolean ret;
++	gint rc;
++	g_autofree gchar *conf_dir = NULL;
++	g_autofree gchar *conf_file = NULL;
++	g_autofree gchar *fn = NULL;
++	g_autofree gchar *testdatadir = NULL;
++	g_autofree gchar *value = NULL;
++	g_autoptr(FuPlugin) plugin = fu_plugin_new(NULL);
++	g_autoptr(GError) error = NULL;
++
++	/* this is a build file */
++	testdatadir = g_test_build_filename(G_TEST_BUILT, "tests", NULL);
++	(void)g_setenv("FWUPD_SYSCONFDIR", testdatadir, TRUE);
++	conf_dir = fu_path_from_kind(FU_PATH_KIND_SYSCONFDIR_PKG);
++
++	/* remove existing file */
++	fu_plugin_set_name(plugin, "test");
++	conf_file = g_strdup_printf("%s.conf", fu_plugin_get_name(plugin));
++	fn = g_build_filename(conf_dir, conf_file, NULL);
++	ret = fu_path_mkdir_parent(fn, &error);
++	g_assert_no_error(error);
++	g_assert_true(ret);
++	g_remove(fn);
++	ret = g_file_set_contents(fn, "", -1, &error);
++	g_assert_no_error(error);
++	g_assert_true(ret);
++
++	/* set a value */
++	ret = fu_plugin_set_config_value(plugin, "Key", "True", &error);
++	g_assert_no_error(error);
++	g_assert_true(ret);
++	g_assert_true(g_file_test(fn, G_FILE_TEST_EXISTS));
++
++	/* check it is world readable */
++	rc = g_stat(fn, &statbuf);
++	g_assert_cmpint(rc, ==, 0);
++	g_assert_cmpint(statbuf.st_mode & 0777, ==, 0644);
++
++	/* read back the value */
++	value = fu_plugin_get_config_value(plugin, "Key");
++	g_assert_cmpstr(value, ==, "True");
++	g_assert_true(fu_plugin_get_config_value_boolean(plugin, "Key"));
++
++	/* check it is private, i.e. only readable by the user/group */
++	ret = fu_plugin_set_secure_config_value(plugin, "Key", "False", &error);
++	g_assert_no_error(error);
++	g_assert_true(ret);
++	rc = g_stat(fn, &statbuf);
++	g_assert_cmpint(rc, ==, 0);
++	g_assert_cmpint(statbuf.st_mode & 0777, ==, 0640);
++}
++
+ static void
+ fu_plugin_devices_func(void)
+ {
+@@ -3598,6 +3654,7 @@ main(int argc, char **argv)
+ 	g_test_add_func("/fwupd/progress{finish}", fu_progress_finish_func);
+ 	g_test_add_func("/fwupd/bios-attrs{load}", fu_bios_settings_load_func);
+ 	g_test_add_func("/fwupd/security-attrs{hsi}", fu_security_attrs_hsi_func);
++	g_test_add_func("/fwupd/plugin{config}", fu_plugin_config_func);
+ 	g_test_add_func("/fwupd/plugin{devices}", fu_plugin_devices_func);
+ 	g_test_add_func("/fwupd/plugin{device-inhibit-children}",
+ 			fu_plugin_device_inhibit_children_func);
diff --git a/meta-oe/recipes-bsp/fwupd/fwupd_1.8.4.bb b/meta-oe/recipes-bsp/fwupd/fwupd_1.8.4.bb
index 99077923dc..794a678833 100644
--- a/meta-oe/recipes-bsp/fwupd/fwupd_1.8.4.bb
+++ b/meta-oe/recipes-bsp/fwupd/fwupd_1.8.4.bb
@@ -6,7 +6,9 @@  DEPENDS = "glib-2.0 libxmlb json-glib libjcat gcab vala-native"
 
 SRC_URI = "https://github.com/${BPN}/${BPN}/releases/download/${PV}/${BP}.tar.xz \
            file://c54ae9c524998e449b822feb465a0c90317cd735.patch \
-           file://run-ptest"
+           file://run-ptest \
+           file://CVE-2022-3287.patch \
+           "
 SRC_URI[sha256sum] = "adfa07434cdc29ec41c40fef460e8d970963fe0c7e849dec7f3932adb161f886"
 
 UPSTREAM_CHECK_URI = "https://github.com/${BPN}/${BPN}/releases"