[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1049862: marked as done (bookworm-pu: package efibootguard/0.13-2+deb12u1)



Your message dated Sat, 07 Oct 2023 09:59:39 +0000
with message-id <E1qp45z-00A4Ca-Pk@coccia.debian.org>
and subject line Released with 12.2
has caused the Debian Bug report #1049862,
regarding bookworm-pu: package efibootguard/0.13-2+deb12u1
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
1049862: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1049862
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Control: affects -1 + src:efibootguard
X-Debbugs-Cc: efibootguard@packages.debian.org
User: release.debian.org@packages.debian.org
Usertags: pu
Tags: bookworm
Severity: normal

[ Reason ]
This backports the fix for CVE-2023-39950 to bookworm.
The Security Team told us to go the stable-pu route.

[ Impact ]
The user might be vulnerable to CVE-2023-39950 in certain
configurations. This will be some swupdate users in Debian.

[ Tests ]
I did not exploit the bug (no time for this).
I checked that the patches compile okay.

[ Risks ]
None.

[ Checklist ]
  [x] *all* changes are documented in the d/changelog
  [x] I reviewed all changes and I approve them
  [x] attach debdiff against the package in stable
  [x] the issue is verified as fixed in unstable
diff -Nru efibootguard-0.13/debian/changelog efibootguard-0.13/debian/changelog
--- efibootguard-0.13/debian/changelog	2022-12-20 23:38:11.000000000 +0100
+++ efibootguard-0.13/debian/changelog	2023-08-10 14:51:14.000000000 +0200
@@ -1,3 +1,14 @@
+efibootguard (0.13-2+deb12u1) bookworm; urgency=medium
+
+  * d/patches: Backport fix to address CVE-2023-39950
+    Backport of security fix for CVE-2023-39950, Insufficient
+    or missing validation and sanitization of input from untrustworthy
+    bootloader environment files can cause crashes and probably also
+    code injections into `bg_setenv`) or programs using `libebgenv`.
+    (Closes: #1049436)
+
+ -- Gylstorff Quirin <quirin.gylstorff@siemens.com>  Thu, 10 Aug 2023 14:51:14 +0200
+
 efibootguard (0.13-2) unstable; urgency=medium
 
     * use correct version to break/replaces libebgenv-dev 0.12-1
diff -Nru efibootguard-0.13/debian/patches/Introduce-validation-of-bgenv-prior-to-its-usage.patch efibootguard-0.13/debian/patches/Introduce-validation-of-bgenv-prior-to-its-usage.patch
--- efibootguard-0.13/debian/patches/Introduce-validation-of-bgenv-prior-to-its-usage.patch	1970-01-01 01:00:00.000000000 +0100
+++ efibootguard-0.13/debian/patches/Introduce-validation-of-bgenv-prior-to-its-usage.patch	2023-08-10 14:51:14.000000000 +0200
@@ -0,0 +1,190 @@
+From 188fe5f47f9f9e8a4f67bf4e4a840ce84d80641c Mon Sep 17 00:00:00 2001
+From: Jan Kiszka <jan.kiszka@siemens.com>
+Date: Mon, 24 Jul 2023 08:00:34 +0200
+Subject: [PATCH 5/9] Introduce validation of bgenv prior to its usage
+
+The parsing of user variables assumes sane input so far and can be
+mislead to out-of-bounds accesses, including writes. Address this by
+always validating a bgenv after reading it from a partition or a file.
+If an invalid bgenv is found, it is cleared to zero internally so that
+the existing code will always operate against a sane state.
+
+Include the CRC32 validation in the new helper as well which also
+ensures that the checksum is tested when operating against a specific
+file.
+
+Reported by Code Intelligence.
+
+Addresses CVE-2023-39950
+
+Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
+---
+ env/env_api_fat.c   | 44 ++++++++++++++++++++++++++++++++++----------
+ env/uservars.c      | 29 +++++++++++++++++++++++++++++
+ include/env_api.h   |  2 ++
+ include/uservars.h  |  3 +++
+ tools/bg_envtools.c |  6 +++++-
+ 5 files changed, 73 insertions(+), 11 deletions(-)
+
+diff --git a/env/env_api_fat.c b/env/env_api_fat.c
+index 0f4f474..b7540bb 100644
+--- a/env/env_api_fat.c
++++ b/env/env_api_fat.c
+@@ -51,6 +51,33 @@ void bgenv_be_verbose(bool v)
+ 	ebgpart_beverbose(v);
+ }
+ 
++static void clear_envdata(BG_ENVDATA *data)
++{
++	memset(data, 0, sizeof(BG_ENVDATA));
++	data->crc32 = crc32(0, (Bytef *)data,
++			    sizeof(BG_ENVDATA) - sizeof(data->crc32));
++}
++
++bool validate_envdata(BG_ENVDATA *data)
++{
++	uint32_t sum = crc32(0, (Bytef *)data,
++			     sizeof(BG_ENVDATA) - sizeof(data->crc32));
++
++	if (data->crc32 != sum) {
++		VERBOSE(stderr, "Invalid CRC32!\n");
++		/* clear invalid environment */
++		clear_envdata(data);
++		return false;
++	}
++	if (!bgenv_validate_uservars(data->userdata)) {
++		VERBOSE(stderr, "Corrupt uservars!\n");
++		/* clear invalid environment */
++		clear_envdata(data);
++		return false;
++	}
++	return true;
++}
++
+ bool read_env(CONFIG_PART *part, BG_ENVDATA *env)
+ {
+ 	if (!part) {
+@@ -86,10 +113,16 @@ bool read_env(CONFIG_PART *part, BG_ENVDATA *env)
+ 	if (part->not_mounted) {
+ 		unmount_partition(part);
+ 	}
++	if (result == false) {
++		clear_envdata(env);
++		return false;
++	}
++
+ 	/* enforce NULL-termination of strings */
+ 	env->kernelfile[ENV_STRING_LENGTH - 1] = 0;
+ 	env->kernelparams[ENV_STRING_LENGTH - 1] = 0;
+-	return result;
++
++	return validate_envdata(env);
+ }
+ 
+ bool write_env(CONFIG_PART *part, BG_ENVDATA *env)
+@@ -147,15 +180,6 @@ bool bgenv_init(void)
+ 	}
+ 	for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ 		read_env(&config_parts[i], &envdata[i]);
+-		uint32_t sum = crc32(0, (Bytef *)&envdata[i],
+-		    sizeof(BG_ENVDATA) - sizeof(envdata[i].crc32));
+-		if (envdata[i].crc32 != sum) {
+-			VERBOSE(stderr, "Invalid CRC32!\n");
+-			/* clear invalid environment */
+-			memset(&envdata[i], 0, sizeof(BG_ENVDATA));
+-			envdata[i].crc32 = crc32(0, (Bytef *)&envdata[i],
+-			    sizeof(BG_ENVDATA) - sizeof(envdata[i].crc32));
+-		}
+ 	}
+ 	initialized = true;
+ 	return true;
+diff --git a/env/uservars.c b/env/uservars.c
+index f251f24..23a6920 100644
+--- a/env/uservars.c
++++ b/env/uservars.c
+@@ -72,6 +72,35 @@ void bgenv_map_uservar(uint8_t *udata, char **key, uint64_t *type, uint8_t **val
+ 	}
+ }
+ 
++bool bgenv_validate_uservars(uint8_t *udata)
++{
++	uint32_t spaceleft = ENV_MEM_USERVARS;
++
++	while (*udata) {
++		uint32_t key_len = strnlen((char *)udata, spaceleft);
++
++		/* we need space for the key string + null termination +
++		 * the payload size field */
++		if (key_len + 1 + sizeof(uint32_t) >= spaceleft) {
++			return false;
++		}
++
++		spaceleft -= key_len + 1;
++		udata += key_len + 1;
++
++		uint32_t payload_size = *(uint32_t *)udata;
++
++		/* the payload must leave at least one byte free */
++		if (payload_size >= spaceleft) {
++			return false;
++		}
++
++		spaceleft -= payload_size;
++		udata += payload_size;
++	}
++	return true;
++}
++
+ void bgenv_serialize_uservar(uint8_t *p, char *key, uint64_t type, void *data,
+ 			    uint32_t record_size)
+ {
+diff --git a/include/env_api.h b/include/env_api.h
+index b796682..02b4167 100644
+--- a/include/env_api.h
++++ b/include/env_api.h
+@@ -95,3 +95,5 @@ extern int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
+ extern int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
+ 		     uint32_t datalen);
+ extern uint8_t *bgenv_find_uservar(uint8_t *userdata, char *key);
++
++extern bool validate_envdata(BG_ENVDATA *data);
+diff --git a/include/uservars.h b/include/uservars.h
+index f2f3587..28d6502 100644
+--- a/include/uservars.h
++++ b/include/uservars.h
+@@ -14,6 +14,7 @@
+ 
+ #pragma once
+ 
++#include <stdbool.h>
+ #include <stdint.h>
+ 
+ void bgenv_map_uservar(uint8_t *udata, char **key, uint64_t *type,
+@@ -35,3 +36,5 @@ uint8_t *bgenv_uservar_realloc(uint8_t *udata, uint32_t new_rsize,
+ 			       uint8_t *p);
+ void bgenv_del_uservar(uint8_t *udata, uint8_t *var);
+ uint32_t bgenv_user_free(uint8_t *udata);
++
++bool bgenv_validate_uservars(uint8_t *udata);
+diff --git a/tools/bg_envtools.c b/tools/bg_envtools.c
+index 4d3cfa8..786d6c1 100644
+--- a/tools/bg_envtools.c
++++ b/tools/bg_envtools.c
+@@ -155,9 +155,13 @@ bool get_env(char *configfilepath, BG_ENVDATA *data)
+ 			"Error closing environment file after reading.\n");
+ 	};
+ 
++	if (result == false) {
++		return false;
++	}
++
+ 	/* enforce NULL-termination of strings */
+ 	data->kernelfile[ENV_STRING_LENGTH - 1] = 0;
+ 	data->kernelparams[ENV_STRING_LENGTH - 1] = 0;
+ 
+-	return result;
++	return validate_envdata(data);
+ }
+-- 
+2.40.1
+
diff -Nru efibootguard-0.13/debian/patches/series efibootguard-0.13/debian/patches/series
--- efibootguard-0.13/debian/patches/series	2022-12-20 23:38:11.000000000 +0100
+++ efibootguard-0.13/debian/patches/series	2023-08-10 14:51:14.000000000 +0200
@@ -1,2 +1,4 @@
 Do-not-use-ld-fatal-warnings.patch
 Prevent-reading-version-from-git.patch
+tools-Ensure-that-kernelfile-and-kernelparams-are-nu.patch
+Introduce-validation-of-bgenv-prior-to-its-usage.patch
diff -Nru efibootguard-0.13/debian/patches/tools-Ensure-that-kernelfile-and-kernelparams-are-nu.patch efibootguard-0.13/debian/patches/tools-Ensure-that-kernelfile-and-kernelparams-are-nu.patch
--- efibootguard-0.13/debian/patches/tools-Ensure-that-kernelfile-and-kernelparams-are-nu.patch	1970-01-01 01:00:00.000000000 +0100
+++ efibootguard-0.13/debian/patches/tools-Ensure-that-kernelfile-and-kernelparams-are-nu.patch	2023-08-10 14:51:14.000000000 +0200
@@ -0,0 +1,36 @@
+From 8d241307a5b736b4802251ba9bd021efdc089f42 Mon Sep 17 00:00:00 2001
+From: Jan Kiszka <jan.kiszka@siemens.com>
+Date: Mon, 24 Jul 2023 07:04:09 +0200
+Subject: [PATCH 4/9] tools: Ensure that kernelfile and kernelparams are
+ null-terminated
+
+Analogously to read_env(), ensure also when reading an environment from
+a specified file that those statically sized strings are properly
+terminated before accessing them. Prevents potential out-of-bounds read
+accesses in bg_printenv or bg_setenv.
+
+Addresses CVE-2023-39950
+
+Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
+---
+ tools/bg_envtools.c | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/tools/bg_envtools.c b/tools/bg_envtools.c
+index b81ffed..4d3cfa8 100644
+--- a/tools/bg_envtools.c
++++ b/tools/bg_envtools.c
+@@ -154,5 +154,10 @@ bool get_env(char *configfilepath, BG_ENVDATA *data)
+ 		VERBOSE(stderr,
+ 			"Error closing environment file after reading.\n");
+ 	};
++
++	/* enforce NULL-termination of strings */
++	data->kernelfile[ENV_STRING_LENGTH - 1] = 0;
++	data->kernelparams[ENV_STRING_LENGTH - 1] = 0;
++
+ 	return result;
+ }
+-- 
+2.40.1
+

--- End Message ---
--- Begin Message ---
Version: 12.2

The upload requested in this bug has been released as part of 12.2.

--- End Message ---

Reply to: