1From 83603bea6ce8fdff5ab3fbc4c9e592a8c71a8706 Mon Sep 17 00:00:00 2001 2From: Thomas Frauendorfer | Miray Software <tf@miray.de> 3Date: Thu, 4 Feb 2021 19:02:33 +0100 4Subject: [PATCH] kern/misc: Add function to check printf() format against 5 expected format 6 7The grub_printf_fmt_check() function parses the arguments of an untrusted 8printf() format and an expected printf() format and then compares the 9arguments counts and arguments types. The arguments count in the untrusted 10format string must be less or equal to the arguments count in the expected 11format string and both arguments types must match. 12 13To do this the parse_printf_arg_fmt() helper function is extended in the 14following way: 15 16 1. Add a return value to report errors to the grub_printf_fmt_check(). 17 18 2. Add the fmt_check argument to enable stricter format verification: 19 - the function expects that arguments definitions are always 20 terminated by a supported conversion specifier. 21 - positional parameters, "$", are not allowed, as they cannot be 22 validated correctly with the current implementation. For example 23 "%s%1$d" would assign the first args entry twice while leaving the 24 second one unchanged. 25 - Return an error if preallocated space in args is too small and 26 allocation fails for the needed size. The grub_printf_fmt_check() 27 should verify all arguments. So, if validation is not possible for 28 any reason it should return an error. 29 This also adds a case entry to handle "%%", which is the escape 30 sequence to print "%" character. 31 32 3. Add the max_args argument to check for the maximum allowed arguments 33 count in a printf() string. This should be set to the arguments count 34 of the expected format. Then the parse_printf_arg_fmt() function will 35 return an error if the arguments count is exceeded. 36 37The two additional arguments allow us to use parse_printf_arg_fmt() in 38printf() and grub_printf_fmt_check() calls. 39 40When parse_printf_arg_fmt() is used by grub_printf_fmt_check() the 41function parse user provided untrusted format string too. So, in 42that case it is better to be too strict than too lenient. 43 44Signed-off-by: Thomas Frauendorfer | Miray Software <tf@miray.de> 45Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> 46Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com> 47--- 48 grub-core/kern/misc.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++--- 49 include/grub/misc.h | 16 ++++++++++ 50 2 files changed, 94 insertions(+), 4 deletions(-) 51 52diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c 53index 22417f7..90317b6 100644 54--- a/grub-core/kern/misc.c 55+++ b/grub-core/kern/misc.c 56@@ -632,8 +632,26 @@ grub_lltoa (char *str, int c, unsigned long long n) 57 return p; 58 } 59 60-static void 61-parse_printf_arg_fmt (const char *fmt0, struct printf_args *args) 62+/* 63+ * Parse printf() fmt0 string into args arguments. 64+ * 65+ * The parsed arguments are either used by a printf() function to format the fmt0 66+ * string or they are used to compare a format string from an untrusted source 67+ * against a format string with expected arguments. 68+ * 69+ * When the fmt_check is set to !0, e.g. 1, then this function is executed in 70+ * printf() format check mode. This enforces stricter rules for parsing the 71+ * fmt0 to limit exposure to possible errors in printf() handling. It also 72+ * disables positional parameters, "$", because some formats, e.g "%s%1$d", 73+ * cannot be validated with the current implementation. 74+ * 75+ * The max_args allows to set a maximum number of accepted arguments. If the fmt0 76+ * string defines more arguments than the max_args then the parse_printf_arg_fmt() 77+ * function returns an error. This is currently used for format check only. 78+ */ 79+static grub_err_t 80+parse_printf_arg_fmt (const char *fmt0, struct printf_args *args, 81+ int fmt_check, grub_size_t max_args) 82 { 83 const char *fmt; 84 char c; 85@@ -660,7 +678,12 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args *args) 86 fmt++; 87 88 if (*fmt == '$') 89- fmt++; 90+ { 91+ if (fmt_check) 92+ return grub_error (GRUB_ERR_BAD_ARGUMENT, 93+ "positional arguments are not supported"); 94+ fmt++; 95+ } 96 97 if (*fmt =='-') 98 fmt++; 99@@ -691,9 +714,19 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args *args) 100 case 's': 101 args->count++; 102 break; 103+ case '%': 104+ /* "%%" is the escape sequence to output "%". */ 105+ break; 106+ default: 107+ if (fmt_check) 108+ return grub_error (GRUB_ERR_BAD_ARGUMENT, "unexpected format"); 109+ break; 110 } 111 } 112 113+ if (fmt_check && args->count > max_args) 114+ return grub_error (GRUB_ERR_BAD_ARGUMENT, "too many arguments"); 115+ 116 if (args->count <= ARRAY_SIZE (args->prealloc)) 117 args->ptr = args->prealloc; 118 else 119@@ -701,6 +734,9 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args *args) 120 args->ptr = grub_calloc (args->count, sizeof (args->ptr[0])); 121 if (!args->ptr) 122 { 123+ if (fmt_check) 124+ return grub_errno; 125+ 126 grub_errno = GRUB_ERR_NONE; 127 args->ptr = args->prealloc; 128 args->count = ARRAY_SIZE (args->prealloc); 129@@ -791,6 +827,8 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args *args) 130 break; 131 } 132 } 133+ 134+ return GRUB_ERR_NONE; 135 } 136 137 static void 138@@ -798,7 +836,7 @@ parse_printf_args (const char *fmt0, struct printf_args *args, va_list args_in) 139 { 140 grub_size_t n; 141 142- parse_printf_arg_fmt (fmt0, args); 143+ parse_printf_arg_fmt (fmt0, args, 0, 0); 144 145 for (n = 0; n < args->count; n++) 146 switch (args->ptr[n].type) 147@@ -1105,6 +1143,42 @@ grub_xasprintf (const char *fmt, ...) 148 return ret; 149 } 150 151+grub_err_t 152+grub_printf_fmt_check (const char *fmt, const char *fmt_expected) 153+{ 154+ struct printf_args args_expected, args_fmt; 155+ grub_err_t ret; 156+ grub_size_t n; 157+ 158+ if (fmt == NULL || fmt_expected == NULL) 159+ return grub_error (GRUB_ERR_BAD_ARGUMENT, "invalid format"); 160+ 161+ ret = parse_printf_arg_fmt (fmt_expected, &args_expected, 1, GRUB_SIZE_MAX); 162+ if (ret != GRUB_ERR_NONE) 163+ return ret; 164+ 165+ /* Limit parsing to the number of expected arguments. */ 166+ ret = parse_printf_arg_fmt (fmt, &args_fmt, 1, args_expected.count); 167+ if (ret != GRUB_ERR_NONE) 168+ { 169+ free_printf_args (&args_expected); 170+ return ret; 171+ } 172+ 173+ for (n = 0; n < args_fmt.count; n++) 174+ if (args_fmt.ptr[n].type != args_expected.ptr[n].type) 175+ { 176+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "arguments types do not match"); 177+ break; 178+ } 179+ 180+ free_printf_args (&args_expected); 181+ free_printf_args (&args_fmt); 182+ 183+ return ret; 184+} 185+ 186+ 187 /* Abort GRUB. This function does not return. */ 188 static void __attribute__ ((noreturn)) 189 grub_abort (void) 190diff --git a/include/grub/misc.h b/include/grub/misc.h 191index ee48eb7..d1c5709 100644 192--- a/include/grub/misc.h 193+++ b/include/grub/misc.h 194@@ -440,6 +440,22 @@ grub_error_load (const struct grub_error_saved *save) 195 grub_errno = save->grub_errno; 196 } 197 198+/* 199+ * grub_printf_fmt_checks() a fmt string for printf() against an expected 200+ * format. It is intended for cases where the fmt string could come from 201+ * an outside source and cannot be trusted. 202+ * 203+ * While expected fmt accepts a printf() format string it should be kept 204+ * as simple as possible. The printf() format strings with positional 205+ * parameters are NOT accepted, neither for fmt nor for fmt_expected. 206+ * 207+ * The fmt is accepted if it has equal or less arguments than fmt_expected 208+ * and if the type of all arguments match. 209+ * 210+ * Returns GRUB_ERR_NONE if fmt is acceptable. 211+ */ 212+grub_err_t EXPORT_FUNC (grub_printf_fmt_check) (const char *fmt, const char *fmt_expected); 213+ 214 #if BOOT_TIME_STATS 215 struct grub_boot_time 216 { 217-- 2182.14.2 219 220