| From 4ea7bae51f97e49c84dc67ea30b466ca8633b9f6 Mon Sep 17 00:00:00 2001 |
| From: Chris Coulson <chris.coulson@canonical.com> |
| 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 <chris.coulson@canonical.com> |
| Signed-off-by: Chris Coulson <chris.coulson@canonical.com> |
| Signed-off-by: Darren Kenny <darren.kenny@oracle.com> |
| Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> |
| Signed-off-by: Stefan SΓΈrensen <stefan.sorensen@spectralink.com> |
| --- |
| 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 <grub/parser.h> |
| +#include <grub/buffer.h> |
| #include <grub/env.h> |
| #include <grub/misc.h> |
| #include <grub/mm.h> |
| @@ -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 |
| |