hush: fix a memory corruption when exported variable is modified

The construct such as this:

t=1
export t
t=new_value1

had a small probability of momentarily using free()d value.

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

diff --git a/shell/hush.c b/shell/hush.c
index d0d983018cfa3cfc4623db1d02c699a2ba648cb6..668b1f2b7b2fcf12e9e4259934539ad04808d06a 100644 (file)
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -1891,6 +1891,7 @@ static int set_local_var(char *str, int flg_export, int local_lvl, int flg_read_
 {
        struct variable **var_pp;
        struct variable *cur;
+       char *free_me = NULL;
        char *eq_sign;
        int name_len;
 
@@ -1907,6 +1908,7 @@ static int set_local_var(char *str, int flg_export, int local_lvl, int flg_read_
                        var_pp = &cur->next;
                        continue;
                }
+
                /* We found an existing var with this name */
                if (cur->flg_read_only) {
 #if !BB_MMU
@@ -1955,12 +1957,17 @@ static int set_local_var(char *str, int flg_export, int local_lvl, int flg_read_
                                strcpy(cur->varstr, str);
                                goto free_and_exp;
                        }
-               } else {
-                       /* max_len == 0 signifies "malloced" var, which we can
-                        * (and has to) free */
-                       free(cur->varstr);
-               }
-               cur->max_len = 0;
+                       /* Can't reuse */
+                       cur->max_len = 0;
+                       goto set_str_and_exp;
+               }
+               /* max_len == 0 signifies "malloced" var, which we can
+                * (and have to) free. But we can't free(cur->varstr) here:
+                * if cur->flg_export is 1, it is in the environment.
+                * We should either unsetenv+free, or wait until putenv,
+                * then putenv(new)+free(old).
+                */
+               free_me = cur->varstr;
                goto set_str_and_exp;
        }
 
@@ -1987,10 +1994,15 @@ static int set_local_var(char *str, int flg_export, int local_lvl, int flg_read_
                        cur->flg_export = 0;
                        /* unsetenv was already done */
                } else {
+                       int i;
                        debug_printf_env("%s: putenv '%s'\n", __func__, cur->varstr);
-                       return putenv(cur->varstr);
+                       i = putenv(cur->varstr);
+                       /* only now we can free old exported malloced string */
+                       free(free_me);
+                       return i;
                }
        }
+       free(free_me);
        return 0;
 }