123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219 |
- From 83603bea6ce8fdff5ab3fbc4c9e592a8c71a8706 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>
- Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.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 22417f7..90317b6 100644
- --- a/grub-core/kern/misc.c
- +++ b/grub-core/kern/misc.c
- @@ -632,8 +632,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;
- @@ -660,7 +678,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++;
- @@ -691,9 +714,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
- @@ -701,6 +734,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);
- @@ -791,6 +827,8 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args *args)
- break;
- }
- }
- +
- + return GRUB_ERR_NONE;
- }
-
- static void
- @@ -798,7 +836,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)
- @@ -1105,6 +1143,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 void __attribute__ ((noreturn))
- grub_abort (void)
- diff --git a/include/grub/misc.h b/include/grub/misc.h
- index ee48eb7..d1c5709 100644
- --- a/include/grub/misc.h
- +++ b/include/grub/misc.h
- @@ -440,6 +440,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
- {
- --
- 2.14.2
|