From 26349fcf80982b4d0120b73b2836e88bcf16853c Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Fri, 10 Jul 2020 14:41:45 +0100 Subject: [PATCH] script: Avoid a use-after-free when redefining a function during execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Defining a new function with the same name as a previously defined function causes the grub_script and associated resources for the previous function to be freed. If the previous function is currently executing when a function with the same name is defined, this results in use-after-frees when processing subsequent commands in the original function. Instead, reject a new function definition if it has the same name as a previously defined function, and that function is currently being executed. Although a behavioural change, this should be backwards compatible with existing configurations because they can't be dependent on the current behaviour without being broken. Fixes: CVE-2020-15706 Signed-off-by: Chris Coulson Reviewed-by: Daniel Kiper Signed-off-by: Stefan Sørensen --- grub-core/script/execute.c | 2 ++ grub-core/script/function.c | 16 +++++++++++++--- grub-core/script/parser.y | 3 ++- include/grub/script_sh.h | 2 ++ 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/grub-core/script/execute.c b/grub-core/script/execute.c index c8d6806fe..7e028e135 100644 --- a/grub-core/script/execute.c +++ b/grub-core/script/execute.c @@ -838,7 +838,9 @@ grub_script_function_call (grub_script_function_t func, int argc, char **args) old_scope = scope; scope = &new_scope; + func->executing++; ret = grub_script_execute (func->func); + func->executing--; function_return = 0; active_loops = loops; diff --git a/grub-core/script/function.c b/grub-core/script/function.c index d36655e51..3aad04bf9 100644 --- a/grub-core/script/function.c +++ b/grub-core/script/function.c @@ -34,6 +34,7 @@ grub_script_function_create (struct grub_script_arg *functionname_arg, func = (grub_script_function_t) grub_malloc (sizeof (*func)); if (! func) return 0; + func->executing = 0; func->name = grub_strdup (functionname_arg->str); if (! func->name) @@ -60,10 +61,19 @@ grub_script_function_create (struct grub_script_arg *functionname_arg, grub_script_function_t q; q = *p; - grub_script_free (q->func); - q->func = cmd; grub_free (func); - func = q; + if (q->executing > 0) + { + grub_error (GRUB_ERR_BAD_ARGUMENT, + N_("attempt to redefine a function being executed")); + func = NULL; + } + else + { + grub_script_free (q->func); + q->func = cmd; + func = q; + } } else { diff --git a/grub-core/script/parser.y b/grub-core/script/parser.y index 4f0ab8319..f80b86b6f 100644 --- a/grub-core/script/parser.y +++ b/grub-core/script/parser.y @@ -289,7 +289,8 @@ function: "function" "name" grub_script_mem_free (state->func_mem); else { script->children = state->scripts; - grub_script_function_create ($2, script); + if (!grub_script_function_create ($2, script)) + grub_script_free (script); } state->scripts = $3; diff --git a/include/grub/script_sh.h b/include/grub/script_sh.h index b382bcf09..6c48e0751 100644 --- a/include/grub/script_sh.h +++ b/include/grub/script_sh.h @@ -361,6 +361,8 @@ struct grub_script_function /* The next element. */ struct grub_script_function *next; + + unsigned executing; }; typedef struct grub_script_function *grub_script_function_t; -- 2.26.2