From dae82f6bd72cf600e5d48046ec674a441d0f49d7 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Wed, 2 Aug 2023 14:36:09 -0400
Subject: [PATCH] Further mitigations against CVE-2023-40546 as a class

In CVE-2023-40546, an incorrect invocation of LogError()
causes a read from the page at address 0, which on newer systems will
correctly cause a fault.  The immediate fix for this CVE is to fix the
invocation so that the error is logged correctly, but there is more that
can be done.

This patch adds additional checks to ensure that the format specifier on
any of these invocations can not be NULL, thereby mitigating this entire
class of error from creating a fault.  Additionally, most of these
checks are done using _Static_assert(), so they should normally be
triggered at compile time.

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 errlog.c |  3 +++
 shim.h   | 26 ++++++++++++++++++++------
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/errlog.c b/errlog.c
index cc6a89f..3c5e0af 100644
--- a/errlog.c
+++ b/errlog.c
@@ -32,6 +32,9 @@ VLogError(const char *file, int line, const char *func, const CHAR16 *fmt,
 	ms_va_list args2;
 	CHAR16 **newerrs;
 
+	if (file == NULL || func == NULL || fmt == NULL)
+		return EFI_INVALID_PARAMETER;
+
 	newerrs = ReallocatePool(errs, (nerrs + 1) * sizeof(*errs),
 				       (nerrs + 3) * sizeof(*errs));
 	if (!newerrs)
diff --git a/shim.h b/shim.h
index 3e221b5..652be45 100644
--- a/shim.h
+++ b/shim.h
@@ -281,18 +281,32 @@ verify_buffer (char *data, int datasize,
 #ifndef SHIM_UNIT_TEST
 #define perror_(file, line, func, fmt, ...) ({					\
 		UINTN __perror_ret = 0;						\
+		_Static_assert((fmt) != NULL,					\
+			       "format specifier cannot be NULL");		\
 		if (!in_protocol)						\
 			__perror_ret = console_print((fmt), ##__VA_ARGS__);	\
 		LogError_(file, line, func, fmt, ##__VA_ARGS__);		\
 		__perror_ret;							\
 	})
-#define perror(fmt, ...) \
-	perror_(__FILE__, __LINE__ - 1, __func__, fmt, ##__VA_ARGS__)
-#define LogError(fmt, ...) \
-	LogError_(__FILE__, __LINE__ - 1, __func__, fmt, ##__VA_ARGS__)
+#define perror(fmt, ...) ({							\
+		_Static_assert((fmt) != NULL,					\
+			       "format specifier cannot be NULL");		\
+		perror_(__FILE__, __LINE__ - 1, __func__, fmt, ##__VA_ARGS__);	\
+	})
+#define LogError(fmt, ...) ({							\
+		_Static_assert((fmt) != NULL,					\
+			       "format specifier cannot be NULL");		\
+		LogError_(__FILE__, __LINE__ - 1, __func__, fmt, ##__VA_ARGS__);\
+	})
 #else
-#define perror(fmt, ...)
-#define LogError(fmt, ...)
+#define perror(fmt, ...) ({							\
+		_Static_assert((fmt) != NULL,					\
+			       "format specifier cannot be NULL");		\
+	})
+#define LogError(fmt, ...) ({							\
+		_Static_assert((fmt) != NULL,					\
+			       "format specifier cannot be NULL");		\
+	})
 #endif
 
 #ifdef ENABLE_SHIM_DEVEL
-- 
2.33.0