1*4882a593SmuzhiyunFrom 26349fcf80982b4d0120b73b2836e88bcf16853c Mon Sep 17 00:00:00 2001 2*4882a593SmuzhiyunFrom: Chris Coulson <chris.coulson@canonical.com> 3*4882a593SmuzhiyunDate: Fri, 10 Jul 2020 14:41:45 +0100 4*4882a593SmuzhiyunSubject: [PATCH] script: Avoid a use-after-free when redefining a 5*4882a593Smuzhiyun function during execution 6*4882a593SmuzhiyunMIME-Version: 1.0 7*4882a593SmuzhiyunContent-Type: text/plain; charset=UTF-8 8*4882a593SmuzhiyunContent-Transfer-Encoding: 8bit 9*4882a593Smuzhiyun 10*4882a593SmuzhiyunDefining a new function with the same name as a previously defined 11*4882a593Smuzhiyunfunction causes the grub_script and associated resources for the 12*4882a593Smuzhiyunprevious function to be freed. If the previous function is currently 13*4882a593Smuzhiyunexecuting when a function with the same name is defined, this results 14*4882a593Smuzhiyunin use-after-frees when processing subsequent commands in the original 15*4882a593Smuzhiyunfunction. 16*4882a593Smuzhiyun 17*4882a593SmuzhiyunInstead, reject a new function definition if it has the same name as 18*4882a593Smuzhiyuna previously defined function, and that function is currently being 19*4882a593Smuzhiyunexecuted. Although a behavioural change, this should be backwards 20*4882a593Smuzhiyuncompatible with existing configurations because they can't be 21*4882a593Smuzhiyundependent on the current behaviour without being broken. 22*4882a593Smuzhiyun 23*4882a593SmuzhiyunFixes: CVE-2020-15706 24*4882a593Smuzhiyun 25*4882a593SmuzhiyunSigned-off-by: Chris Coulson <chris.coulson@canonical.com> 26*4882a593SmuzhiyunReviewed-by: Daniel Kiper <daniel.kiper@oracle.com> 27*4882a593SmuzhiyunSigned-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com> 28*4882a593Smuzhiyun--- 29*4882a593Smuzhiyun grub-core/script/execute.c | 2 ++ 30*4882a593Smuzhiyun grub-core/script/function.c | 16 +++++++++++++--- 31*4882a593Smuzhiyun grub-core/script/parser.y | 3 ++- 32*4882a593Smuzhiyun include/grub/script_sh.h | 2 ++ 33*4882a593Smuzhiyun 4 files changed, 19 insertions(+), 4 deletions(-) 34*4882a593Smuzhiyun 35*4882a593Smuzhiyundiff --git a/grub-core/script/execute.c b/grub-core/script/execute.c 36*4882a593Smuzhiyunindex c8d6806fe..7e028e135 100644 37*4882a593Smuzhiyun--- a/grub-core/script/execute.c 38*4882a593Smuzhiyun+++ b/grub-core/script/execute.c 39*4882a593Smuzhiyun@@ -838,7 +838,9 @@ grub_script_function_call (grub_script_function_t func, int argc, char **args) 40*4882a593Smuzhiyun old_scope = scope; 41*4882a593Smuzhiyun scope = &new_scope; 42*4882a593Smuzhiyun 43*4882a593Smuzhiyun+ func->executing++; 44*4882a593Smuzhiyun ret = grub_script_execute (func->func); 45*4882a593Smuzhiyun+ func->executing--; 46*4882a593Smuzhiyun 47*4882a593Smuzhiyun function_return = 0; 48*4882a593Smuzhiyun active_loops = loops; 49*4882a593Smuzhiyundiff --git a/grub-core/script/function.c b/grub-core/script/function.c 50*4882a593Smuzhiyunindex d36655e51..3aad04bf9 100644 51*4882a593Smuzhiyun--- a/grub-core/script/function.c 52*4882a593Smuzhiyun+++ b/grub-core/script/function.c 53*4882a593Smuzhiyun@@ -34,6 +34,7 @@ grub_script_function_create (struct grub_script_arg *functionname_arg, 54*4882a593Smuzhiyun func = (grub_script_function_t) grub_malloc (sizeof (*func)); 55*4882a593Smuzhiyun if (! func) 56*4882a593Smuzhiyun return 0; 57*4882a593Smuzhiyun+ func->executing = 0; 58*4882a593Smuzhiyun 59*4882a593Smuzhiyun func->name = grub_strdup (functionname_arg->str); 60*4882a593Smuzhiyun if (! func->name) 61*4882a593Smuzhiyun@@ -60,10 +61,19 @@ grub_script_function_create (struct grub_script_arg *functionname_arg, 62*4882a593Smuzhiyun grub_script_function_t q; 63*4882a593Smuzhiyun 64*4882a593Smuzhiyun q = *p; 65*4882a593Smuzhiyun- grub_script_free (q->func); 66*4882a593Smuzhiyun- q->func = cmd; 67*4882a593Smuzhiyun grub_free (func); 68*4882a593Smuzhiyun- func = q; 69*4882a593Smuzhiyun+ if (q->executing > 0) 70*4882a593Smuzhiyun+ { 71*4882a593Smuzhiyun+ grub_error (GRUB_ERR_BAD_ARGUMENT, 72*4882a593Smuzhiyun+ N_("attempt to redefine a function being executed")); 73*4882a593Smuzhiyun+ func = NULL; 74*4882a593Smuzhiyun+ } 75*4882a593Smuzhiyun+ else 76*4882a593Smuzhiyun+ { 77*4882a593Smuzhiyun+ grub_script_free (q->func); 78*4882a593Smuzhiyun+ q->func = cmd; 79*4882a593Smuzhiyun+ func = q; 80*4882a593Smuzhiyun+ } 81*4882a593Smuzhiyun } 82*4882a593Smuzhiyun else 83*4882a593Smuzhiyun { 84*4882a593Smuzhiyundiff --git a/grub-core/script/parser.y b/grub-core/script/parser.y 85*4882a593Smuzhiyunindex 4f0ab8319..f80b86b6f 100644 86*4882a593Smuzhiyun--- a/grub-core/script/parser.y 87*4882a593Smuzhiyun+++ b/grub-core/script/parser.y 88*4882a593Smuzhiyun@@ -289,7 +289,8 @@ function: "function" "name" 89*4882a593Smuzhiyun grub_script_mem_free (state->func_mem); 90*4882a593Smuzhiyun else { 91*4882a593Smuzhiyun script->children = state->scripts; 92*4882a593Smuzhiyun- grub_script_function_create ($2, script); 93*4882a593Smuzhiyun+ if (!grub_script_function_create ($2, script)) 94*4882a593Smuzhiyun+ grub_script_free (script); 95*4882a593Smuzhiyun } 96*4882a593Smuzhiyun 97*4882a593Smuzhiyun state->scripts = $<scripts>3; 98*4882a593Smuzhiyundiff --git a/include/grub/script_sh.h b/include/grub/script_sh.h 99*4882a593Smuzhiyunindex b382bcf09..6c48e0751 100644 100*4882a593Smuzhiyun--- a/include/grub/script_sh.h 101*4882a593Smuzhiyun+++ b/include/grub/script_sh.h 102*4882a593Smuzhiyun@@ -361,6 +361,8 @@ struct grub_script_function 103*4882a593Smuzhiyun 104*4882a593Smuzhiyun /* The next element. */ 105*4882a593Smuzhiyun struct grub_script_function *next; 106*4882a593Smuzhiyun+ 107*4882a593Smuzhiyun+ unsigned executing; 108*4882a593Smuzhiyun }; 109*4882a593Smuzhiyun typedef struct grub_script_function *grub_script_function_t; 110*4882a593Smuzhiyun 111*4882a593Smuzhiyun-- 112*4882a593Smuzhiyun2.26.2 113*4882a593Smuzhiyun 114