From 4ea7bae51f97e49c84dc67ea30b466ca8633b9f6 Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Thu, 7 Jan 2021 19:21:03 +0000 Subject: [PATCH] kern/parser: Fix a stack buffer overflow grub_parser_split_cmdline() expands variable names present in the supplied command line in to their corresponding variable contents and uses a 1 kiB stack buffer for temporary storage without sufficient bounds checking. If the function is called with a command line that references a variable with a sufficiently large payload, it is possible to overflow the stack buffer via tab completion, corrupt the stack frame and potentially control execution. Fixes: CVE-2020-27749 Reported-by: Chris Coulson Signed-off-by: Chris Coulson Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper Signed-off-by: Stefan Sørensen --- grub-core/kern/parser.c | 110 +++++++++++++++++++++++++++++------------------- 1 file changed, 67 insertions(+), 43 deletions(-) diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c index e010eaa..6ab7aa4 100644 --- a/grub-core/kern/parser.c +++ b/grub-core/kern/parser.c @@ -18,6 +18,7 @@ */ #include +#include #include #include #include @@ -107,8 +108,8 @@ check_varstate (grub_parser_state_t s) } -static void -add_var (char *varname, char **bp, char **vp, +static grub_err_t +add_var (grub_buffer_t varname, grub_buffer_t buf, grub_parser_state_t state, grub_parser_state_t newstate) { const char *val; @@ -116,31 +117,41 @@ add_var (char *varname, char **bp, char **vp, /* Check if a variable was being read in and the end of the name was reached. */ if (!(check_varstate (state) && !check_varstate (newstate))) - return; + return GRUB_ERR_NONE; + + if (grub_buffer_append_char (varname, '\0') != GRUB_ERR_NONE) + return grub_errno; - *((*vp)++) = '\0'; - val = grub_env_get (varname); - *vp = varname; + val = grub_env_get ((const char *) grub_buffer_peek_data (varname)); + grub_buffer_reset (varname); if (!val) - return; + return GRUB_ERR_NONE; /* Insert the contents of the variable in the buffer. */ - for (; *val; val++) - *((*bp)++) = *val; + return grub_buffer_append_data (buf, val, grub_strlen (val)); } -static void -terminate_arg (char *buffer, char **bp, int *argc) +static grub_err_t +terminate_arg (grub_buffer_t buffer, int *argc) { - if (*bp != buffer && *((*bp) - 1) != '\0') - { - *((*bp)++) = '\0'; - (*argc)++; - } + grub_size_t unread = grub_buffer_get_unread_bytes (buffer); + + if (unread == 0) + return GRUB_ERR_NONE; + + if (*(const char *) grub_buffer_peek_data_at (buffer, unread - 1) == '\0') + return GRUB_ERR_NONE; + + if (grub_buffer_append_char (buffer, '\0') != GRUB_ERR_NONE) + return grub_errno; + + (*argc)++; + + return GRUB_ERR_NONE; } static grub_err_t -process_char (char c, char *buffer, char **bp, char *varname, char **vp, +process_char (char c, grub_buffer_t buffer, grub_buffer_t varname, grub_parser_state_t state, int *argc, grub_parser_state_t *newstate) { @@ -153,12 +164,13 @@ process_char (char c, char *buffer, char **bp, char *varname, char **vp, * not describe the variable anymore, write the variable to * the buffer. */ - add_var (varname, bp, vp, state, *newstate); + if (add_var (varname, buffer, state, *newstate) != GRUB_ERR_NONE) + return grub_errno; if (check_varstate (*newstate)) { if (use) - *((*vp)++) = use; + return grub_buffer_append_char (varname, use); } else if (*newstate == GRUB_PARSER_STATE_TEXT && state != GRUB_PARSER_STATE_ESC && grub_isspace (use)) @@ -167,10 +179,10 @@ process_char (char c, char *buffer, char **bp, char *varname, char **vp, * Don't add more than one argument if multiple * spaces are used. */ - terminate_arg (buffer, bp, argc); + return terminate_arg (buffer, argc); } else if (use) - *((*bp)++) = use; + return grub_buffer_append_char (buffer, use); return GRUB_ERR_NONE; } @@ -181,19 +193,22 @@ grub_parser_split_cmdline (const char *cmdline, int *argc, char ***argv) { grub_parser_state_t state = GRUB_PARSER_STATE_TEXT; - /* XXX: Fixed size buffer, perhaps this buffer should be dynamically - allocated. */ - char buffer[1024]; - char *bp = buffer; + grub_buffer_t buffer, varname; char *rd = (char *) cmdline; char *rp = rd; - char varname[200]; - char *vp = varname; - char *args; int i; *argc = 0; *argv = NULL; + + buffer = grub_buffer_new (1024); + if (buffer == NULL) + return grub_errno; + + varname = grub_buffer_new (200); + if (varname == NULL) + goto fail; + do { if (rp == NULL || *rp == '\0') @@ -219,7 +234,7 @@ grub_parser_split_cmdline (const char *cmdline, { grub_parser_state_t newstate; - if (process_char (*rp, buffer, &bp, varname, &vp, state, argc, + if (process_char (*rp, buffer, varname, state, argc, &newstate) != GRUB_ERR_NONE) goto fail; @@ -230,10 +245,12 @@ grub_parser_split_cmdline (const char *cmdline, /* A special case for when the last character was part of a variable. */ - add_var (varname, &bp, &vp, state, GRUB_PARSER_STATE_TEXT); + if (add_var (varname, buffer, state, GRUB_PARSER_STATE_TEXT) != GRUB_ERR_NONE) + goto fail; /* Ensure that the last argument is terminated. */ - terminate_arg (buffer, &bp, argc); + if (terminate_arg (buffer, argc) != GRUB_ERR_NONE) + goto fail; /* If there are no args, then we're done. */ if (!*argc) @@ -242,38 +259,45 @@ grub_parser_split_cmdline (const char *cmdline, goto out; } - /* Reserve memory for the return values. */ - args = grub_malloc (bp - buffer); - if (!args) - goto fail; - grub_memcpy (args, buffer, bp - buffer); - *argv = grub_calloc (*argc + 1, sizeof (char *)); if (!*argv) goto fail; /* The arguments are separated with 0's, setup argv so it points to the right values. */ - bp = args; for (i = 0; i < *argc; i++) { - (*argv)[i] = bp; - while (*bp) - bp++; - bp++; + char *arg; + + if (i > 0) + { + if (grub_buffer_advance_read_pos (buffer, 1) != GRUB_ERR_NONE) + goto fail; + } + + arg = (char *) grub_buffer_peek_data (buffer); + if (arg == NULL || + grub_buffer_advance_read_pos (buffer, grub_strlen (arg)) != GRUB_ERR_NONE) + goto fail; + + (*argv)[i] = arg; } + /* Keep memory for the return values. */ + grub_buffer_take_data (buffer); + grub_errno = GRUB_ERR_NONE; out: if (rd != cmdline) grub_free (rd); + grub_buffer_free (buffer); + grub_buffer_free (varname); return grub_errno; fail: grub_free (*argv); - grub_free (args); goto out; } -- 2.14.2