From b7c544c759e8010d07dadd764d59703f3d839bb3 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 11 Oct 2024 11:11:50 +0200
Subject: [PATCH 1/2] smbios: move validation of SMBIOS table sizes fully into
 get_smbios_table()

We do half a validation currently ourselves (i.e. check the header fits
into the rest of the data), and leave the other half to the
caller (i.e. check the table fits into the rest of the data).

get_smbios_table() is changed to accept the minimum object size and
validates it before returning a table.

Based on a discussion with @anonymix007.
---
 src/boot/efi/vmm.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/boot/efi/vmm.c b/src/boot/efi/vmm.c
index 7d7d0d1382..5d20b3a195 100644
--- a/src/boot/efi/vmm.c
+++ b/src/boot/efi/vmm.c
@@ -213,7 +213,7 @@ static const void *find_smbios_configuration_table(uint64_t *ret_size) {
         return NULL;
 }
 
-static const SmbiosHeader *get_smbios_table(uint8_t type, uint64_t *ret_size_left) {
+static const SmbiosHeader *get_smbios_table(uint8_t type, size_t min_size, uint64_t *ret_size_left) {
         uint64_t size = 0;
         const uint8_t *p = find_smbios_configuration_table(&size);
         if (!p)
@@ -233,6 +233,10 @@ static const SmbiosHeader *get_smbios_table(uint8_t type, uint64_t *ret_size_lef
                         return NULL;
 
                 if (header->type == type) {
+                        /* Table is smaller than the minimum expected size? Refuse */
+                        if (header->length < min_size)
+                                return NULL;
+
                         if (ret_size_left)
                                 *ret_size_left = size;
                         return header; /* Yay! */
@@ -273,8 +277,8 @@ static const SmbiosHeader *get_smbios_table(uint8_t type, uint64_t *ret_size_lef
 
 static bool smbios_in_hypervisor(void) {
         /* Look up BIOS Information (Type 0). */
-        const SmbiosTableType0 *type0 = (const SmbiosTableType0 *) get_smbios_table(0, NULL);
-        if (!type0 || type0->header.length < sizeof(SmbiosTableType0))
+        const SmbiosTableType0 *type0 = (const SmbiosTableType0 *) get_smbios_table(0, sizeof(SmbiosTableType0), /* left= */ NULL);
+        if (!type0)
                 return false;
 
         /* Bit 4 of 2nd BIOS characteristics extension bytes indicates virtualization. */
@@ -295,13 +299,13 @@ const char* smbios_find_oem_string(const char *name) {
 
         assert(name);
 
-        const SmbiosTableType11 *type11 = (const SmbiosTableType11 *) get_smbios_table(11, &left);
-        if (!type11 || type11->header.length < sizeof(SmbiosTableType11))
+        const SmbiosTableType11 *type11 = (const SmbiosTableType11 *) get_smbios_table(11, sizeof(SmbiosTableType11), &left);
+        if (!type11)
                 return NULL;
 
-        assert(left >= type11->header.length);
-
         const char *s = type11->contents;
+
+        assert(left >= type11->header.length); /* get_smbios_table() already validated this */
         left -= type11->header.length;
 
         for (const char *p = s; p < s + left; ) {

From 62f0d851a8ee20067531084567c6f6aa397059e3 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 11 Oct 2024 11:13:27 +0200
Subject: [PATCH 2/2] smbios: make code more readable by introducing a "limit"
 pointer

---
 src/boot/efi/vmm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/boot/efi/vmm.c b/src/boot/efi/vmm.c
index 5d20b3a195..1d04509a1e 100644
--- a/src/boot/efi/vmm.c
+++ b/src/boot/efi/vmm.c
@@ -307,9 +307,10 @@ const char* smbios_find_oem_string(const char *name) {
 
         assert(left >= type11->header.length); /* get_smbios_table() already validated this */
         left -= type11->header.length;
+        const char *limit = s + left;
 
-        for (const char *p = s; p < s + left; ) {
-                const char *e = memchr(p, 0, s + left - p);
+        for (const char *p = s; p < limit; ) {
+                const char *e = memchr(p, 0, limit - p);
                 if (!e || e == p) /* Double NUL byte means we've reached the end of the OEM strings. */
                         break;