Andrew Geissler | 6ce62a2 | 2020-11-30 19:58:47 -0600 | [diff] [blame] | 1 | From c65fc7e75b7b7e880d90766057040011701e97f4 Mon Sep 17 00:00:00 2001 |
| 2 | From: Chris Coulson <chris.coulson@canonical.com> |
| 3 | Date: Fri, 10 Jul 2020 14:41:45 +0100 |
| 4 | Subject: [PATCH 8/9] script: Avoid a use-after-free when redefining a function |
| 5 | during execution |
| 6 | |
| 7 | Defining a new function with the same name as a previously defined |
| 8 | function causes the grub_script and associated resources for the |
| 9 | previous function to be freed. If the previous function is currently |
| 10 | executing when a function with the same name is defined, this results |
| 11 | in use-after-frees when processing subsequent commands in the original |
| 12 | function. |
| 13 | |
| 14 | Instead, reject a new function definition if it has the same name as |
| 15 | a previously defined function, and that function is currently being |
| 16 | executed. Although a behavioural change, this should be backwards |
| 17 | compatible with existing configurations because they can't be |
| 18 | dependent on the current behaviour without being broken. |
| 19 | |
| 20 | Fixes: CVE-2020-15706 |
| 21 | |
| 22 | Signed-off-by: Chris Coulson <chris.coulson@canonical.com> |
| 23 | Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> |
| 24 | |
| 25 | Upstream-Status: Backport |
| 26 | CVE: CVE-2020-15706 |
| 27 | |
| 28 | Reference to upstream patch: |
| 29 | https://git.savannah.gnu.org/cgit/grub.git/commit/?id=426f57383d647406ae9c628c472059c27cd6e040 |
| 30 | |
| 31 | Signed-off-by: Yongxin Liu <yongxin.liu@windriver.com> |
| 32 | --- |
| 33 | grub-core/script/execute.c | 2 ++ |
| 34 | grub-core/script/function.c | 16 +++++++++++++--- |
| 35 | grub-core/script/parser.y | 3 ++- |
| 36 | include/grub/script_sh.h | 2 ++ |
| 37 | 4 files changed, 19 insertions(+), 4 deletions(-) |
| 38 | |
| 39 | diff --git a/grub-core/script/execute.c b/grub-core/script/execute.c |
| 40 | index c8d6806..7e028e1 100644 |
| 41 | --- a/grub-core/script/execute.c |
| 42 | +++ b/grub-core/script/execute.c |
| 43 | @@ -838,7 +838,9 @@ grub_script_function_call (grub_script_function_t func, int argc, char **args) |
| 44 | old_scope = scope; |
| 45 | scope = &new_scope; |
| 46 | |
| 47 | + func->executing++; |
| 48 | ret = grub_script_execute (func->func); |
| 49 | + func->executing--; |
| 50 | |
| 51 | function_return = 0; |
| 52 | active_loops = loops; |
| 53 | diff --git a/grub-core/script/function.c b/grub-core/script/function.c |
| 54 | index d36655e..3aad04b 100644 |
| 55 | --- a/grub-core/script/function.c |
| 56 | +++ b/grub-core/script/function.c |
| 57 | @@ -34,6 +34,7 @@ grub_script_function_create (struct grub_script_arg *functionname_arg, |
| 58 | func = (grub_script_function_t) grub_malloc (sizeof (*func)); |
| 59 | if (! func) |
| 60 | return 0; |
| 61 | + func->executing = 0; |
| 62 | |
| 63 | func->name = grub_strdup (functionname_arg->str); |
| 64 | if (! func->name) |
| 65 | @@ -60,10 +61,19 @@ grub_script_function_create (struct grub_script_arg *functionname_arg, |
| 66 | grub_script_function_t q; |
| 67 | |
| 68 | q = *p; |
| 69 | - grub_script_free (q->func); |
| 70 | - q->func = cmd; |
| 71 | grub_free (func); |
| 72 | - func = q; |
| 73 | + if (q->executing > 0) |
| 74 | + { |
| 75 | + grub_error (GRUB_ERR_BAD_ARGUMENT, |
| 76 | + N_("attempt to redefine a function being executed")); |
| 77 | + func = NULL; |
| 78 | + } |
| 79 | + else |
| 80 | + { |
| 81 | + grub_script_free (q->func); |
| 82 | + q->func = cmd; |
| 83 | + func = q; |
| 84 | + } |
| 85 | } |
| 86 | else |
| 87 | { |
| 88 | diff --git a/grub-core/script/parser.y b/grub-core/script/parser.y |
| 89 | index 4f0ab83..f80b86b 100644 |
| 90 | --- a/grub-core/script/parser.y |
| 91 | +++ b/grub-core/script/parser.y |
| 92 | @@ -289,7 +289,8 @@ function: "function" "name" |
| 93 | grub_script_mem_free (state->func_mem); |
| 94 | else { |
| 95 | script->children = state->scripts; |
| 96 | - grub_script_function_create ($2, script); |
| 97 | + if (!grub_script_function_create ($2, script)) |
| 98 | + grub_script_free (script); |
| 99 | } |
| 100 | |
| 101 | state->scripts = $<scripts>3; |
| 102 | diff --git a/include/grub/script_sh.h b/include/grub/script_sh.h |
| 103 | index b382bcf..6c48e07 100644 |
| 104 | --- a/include/grub/script_sh.h |
| 105 | +++ b/include/grub/script_sh.h |
| 106 | @@ -361,6 +361,8 @@ struct grub_script_function |
| 107 | |
| 108 | /* The next element. */ |
| 109 | struct grub_script_function *next; |
| 110 | + |
| 111 | + unsigned executing; |
| 112 | }; |
| 113 | typedef struct grub_script_function *grub_script_function_t; |
| 114 | |
| 115 | -- |
| 116 | 2.14.4 |
| 117 | |