Package: release.debian.org
Control: affects -1 + src:efibootguard
X-Debbugs-Cc: efibootgu...@packages.debian.org
User: release.debian....@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.gylsto...@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.kis...@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.kis...@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.kis...@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.kis...@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
+
