Blame SOURCES/0477-kern-misc-Add-function-to-check-printf-format-agains.patch

468bd4
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
468bd4
From: Thomas Frauendorfer | Miray Software <tf@miray.de>
468bd4
Date: Thu, 4 Feb 2021 19:02:33 +0100
468bd4
Subject: [PATCH] kern/misc: Add function to check printf() format against
468bd4
 expected format
468bd4
468bd4
The grub_printf_fmt_check() function parses the arguments of an untrusted
468bd4
printf() format and an expected printf() format and then compares the
468bd4
arguments counts and arguments types. The arguments count in the untrusted
468bd4
format string must be less or equal to the arguments count in the expected
468bd4
format string and both arguments types must match.
468bd4
468bd4
To do this the parse_printf_arg_fmt() helper function is extended in the
468bd4
following way:
468bd4
468bd4
  1. Add a return value to report errors to the grub_printf_fmt_check().
468bd4
468bd4
  2. Add the fmt_check argument to enable stricter format verification:
468bd4
     - the function expects that arguments definitions are always
468bd4
       terminated by a supported conversion specifier.
468bd4
     - positional parameters, "$", are not allowed, as they cannot be
468bd4
       validated correctly with the current implementation. For example
468bd4
       "%s%1$d" would assign the first args entry twice while leaving the
468bd4
       second one unchanged.
468bd4
     - Return an error if preallocated space in args is too small and
468bd4
       allocation fails for the needed size. The grub_printf_fmt_check()
468bd4
       should verify all arguments. So, if validation is not possible for
468bd4
       any reason it should return an error.
468bd4
     This also adds a case entry to handle "%%", which is the escape
468bd4
     sequence to print "%" character.
468bd4
468bd4
  3. Add the max_args argument to check for the maximum allowed arguments
468bd4
     count in a printf() string. This should be set to the arguments count
468bd4
     of the expected format. Then the parse_printf_arg_fmt() function will
468bd4
     return an error if the arguments count is exceeded.
468bd4
468bd4
The two additional arguments allow us to use parse_printf_arg_fmt() in
468bd4
printf() and grub_printf_fmt_check() calls.
468bd4
468bd4
When parse_printf_arg_fmt() is used by grub_printf_fmt_check() the
468bd4
function parse user provided untrusted format string too. So, in
468bd4
that case it is better to be too strict than too lenient.
468bd4
468bd4
Signed-off-by: Thomas Frauendorfer | Miray Software <tf@miray.de>
468bd4
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
468bd4
---
468bd4
 grub-core/kern/misc.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++---
468bd4
 include/grub/misc.h   | 16 ++++++++++
468bd4
 2 files changed, 94 insertions(+), 4 deletions(-)
468bd4
468bd4
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
09e3cc
index 07456faa2..859d71659 100644
468bd4
--- a/grub-core/kern/misc.c
468bd4
+++ b/grub-core/kern/misc.c
468bd4
@@ -711,8 +711,26 @@ grub_lltoa (char *str, int c, unsigned long long n)
468bd4
   return p;
468bd4
 }
468bd4
 
468bd4
-static void
468bd4
-parse_printf_arg_fmt (const char *fmt0, struct printf_args *args)
468bd4
+/*
468bd4
+ * Parse printf() fmt0 string into args arguments.
468bd4
+ *
468bd4
+ * The parsed arguments are either used by a printf() function to format the fmt0
468bd4
+ * string or they are used to compare a format string from an untrusted source
468bd4
+ * against a format string with expected arguments.
468bd4
+ *
468bd4
+ * When the fmt_check is set to !0, e.g. 1, then this function is executed in
468bd4
+ * printf() format check mode. This enforces stricter rules for parsing the
468bd4
+ * fmt0 to limit exposure to possible errors in printf() handling. It also
468bd4
+ * disables positional parameters, "$", because some formats, e.g "%s%1$d",
468bd4
+ * cannot be validated with the current implementation.
468bd4
+ *
468bd4
+ * The max_args allows to set a maximum number of accepted arguments. If the fmt0
468bd4
+ * string defines more arguments than the max_args then the parse_printf_arg_fmt()
468bd4
+ * function returns an error. This is currently used for format check only.
468bd4
+ */
468bd4
+static grub_err_t
468bd4
+parse_printf_arg_fmt (const char *fmt0, struct printf_args *args,
468bd4
+		      int fmt_check, grub_size_t max_args)
468bd4
 {
468bd4
   const char *fmt;
468bd4
   char c;
468bd4
@@ -739,7 +757,12 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args *args)
468bd4
 	fmt++;
468bd4
 
468bd4
       if (*fmt == '$')
468bd4
-	fmt++;
468bd4
+	{
468bd4
+	  if (fmt_check)
468bd4
+	    return grub_error (GRUB_ERR_BAD_ARGUMENT,
468bd4
+			       "positional arguments are not supported");
468bd4
+	  fmt++;
468bd4
+	}
468bd4
 
468bd4
       if (*fmt =='-')
468bd4
 	fmt++;
468bd4
@@ -771,9 +794,19 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args *args)
468bd4
 	case 's':
468bd4
 	  args->count++;
468bd4
 	  break;
468bd4
+	case '%':
468bd4
+	  /* "%%" is the escape sequence to output "%". */
468bd4
+	  break;
468bd4
+	default:
468bd4
+	  if (fmt_check)
468bd4
+	    return grub_error (GRUB_ERR_BAD_ARGUMENT, "unexpected format");
468bd4
+	  break;
468bd4
 	}
468bd4
     }
468bd4
 
468bd4
+  if (fmt_check && args->count > max_args)
468bd4
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, "too many arguments");
468bd4
+
468bd4
   if (args->count <= ARRAY_SIZE (args->prealloc))
468bd4
     args->ptr = args->prealloc;
468bd4
   else
468bd4
@@ -781,6 +814,9 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args *args)
468bd4
       args->ptr = grub_calloc (args->count, sizeof (args->ptr[0]));
468bd4
       if (!args->ptr)
468bd4
 	{
468bd4
+	  if (fmt_check)
468bd4
+	    return grub_errno;
468bd4
+
468bd4
 	  grub_errno = GRUB_ERR_NONE;
468bd4
 	  args->ptr = args->prealloc;
468bd4
 	  args->count = ARRAY_SIZE (args->prealloc);
468bd4
@@ -872,6 +908,8 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args *args)
468bd4
 	  break;
468bd4
 	}
468bd4
     }
468bd4
+
468bd4
+  return GRUB_ERR_NONE;
468bd4
 }
468bd4
 
468bd4
 static void
468bd4
@@ -879,7 +917,7 @@ parse_printf_args (const char *fmt0, struct printf_args *args, va_list args_in)
468bd4
 {
468bd4
   grub_size_t n;
468bd4
 
468bd4
-  parse_printf_arg_fmt (fmt0, args);
468bd4
+  parse_printf_arg_fmt (fmt0, args, 0, 0);
468bd4
 
468bd4
   for (n = 0; n < args->count; n++)
468bd4
     switch (args->ptr[n].type)
468bd4
@@ -1187,6 +1225,42 @@ grub_xasprintf (const char *fmt, ...)
468bd4
   return ret;
468bd4
 }
468bd4
 
468bd4
+grub_err_t
468bd4
+grub_printf_fmt_check (const char *fmt, const char *fmt_expected)
468bd4
+{
468bd4
+  struct printf_args args_expected, args_fmt;
468bd4
+  grub_err_t ret;
468bd4
+  grub_size_t n;
468bd4
+
468bd4
+  if (fmt == NULL || fmt_expected == NULL)
468bd4
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, "invalid format");
468bd4
+
468bd4
+  ret = parse_printf_arg_fmt (fmt_expected, &args_expected, 1, GRUB_SIZE_MAX);
468bd4
+  if (ret != GRUB_ERR_NONE)
468bd4
+    return ret;
468bd4
+
468bd4
+  /* Limit parsing to the number of expected arguments. */
468bd4
+  ret = parse_printf_arg_fmt (fmt, &args_fmt, 1, args_expected.count);
468bd4
+  if (ret != GRUB_ERR_NONE)
468bd4
+    {
468bd4
+      free_printf_args (&args_expected);
468bd4
+      return ret;
468bd4
+    }
468bd4
+
468bd4
+  for (n = 0; n < args_fmt.count; n++)
468bd4
+    if (args_fmt.ptr[n].type != args_expected.ptr[n].type)
468bd4
+     {
468bd4
+	ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "arguments types do not match");
468bd4
+	break;
468bd4
+     }
468bd4
+
468bd4
+  free_printf_args (&args_expected);
468bd4
+  free_printf_args (&args_fmt);
468bd4
+
468bd4
+  return ret;
468bd4
+}
468bd4
+
468bd4
+
468bd4
 /* Abort GRUB. This function does not return.  */
468bd4
 static inline void __attribute__ ((noreturn))
468bd4
 grub_abort (void)
468bd4
diff --git a/include/grub/misc.h b/include/grub/misc.h
09e3cc
index 6ca03c4d6..6be6a88f6 100644
468bd4
--- a/include/grub/misc.h
468bd4
+++ b/include/grub/misc.h
468bd4
@@ -488,6 +488,22 @@ grub_error_load (const struct grub_error_saved *save)
468bd4
   grub_errno = save->grub_errno;
468bd4
 }
468bd4
 
468bd4
+/*
468bd4
+ * grub_printf_fmt_checks() a fmt string for printf() against an expected
468bd4
+ * format. It is intended for cases where the fmt string could come from
468bd4
+ * an outside source and cannot be trusted.
468bd4
+ *
468bd4
+ * While expected fmt accepts a printf() format string it should be kept
468bd4
+ * as simple as possible. The printf() format strings with positional
468bd4
+ * parameters are NOT accepted, neither for fmt nor for fmt_expected.
468bd4
+ *
468bd4
+ * The fmt is accepted if it has equal or less arguments than fmt_expected
468bd4
+ * and if the type of all arguments match.
468bd4
+ *
468bd4
+ * Returns GRUB_ERR_NONE if fmt is acceptable.
468bd4
+ */
468bd4
+grub_err_t EXPORT_FUNC (grub_printf_fmt_check) (const char *fmt, const char *fmt_expected);
468bd4
+
468bd4
 #if BOOT_TIME_STATS
468bd4
 struct grub_boot_time
468bd4
 {