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