eliminate a fixed-size buffer in Execute() function.

Signed-off-by: Thomas E. Dickey <dickey@invisible-island.net>

1 files changed, 49 insertions, 22 deletions

diff --git a/src/menus.c b/src/menus.c
index 7ce0e49..8b1cb00 100644
--- a/src/menus.c
+++ b/src/menus.c
@@ -919,7 +919,7 @@ PopUpMenu(MenuRoot *menu, int x, int y, Bool center)
     TwmWindow **WindowNames;
     TwmWindow *tmp_win2, *tmp_win3;
     int i;
-    int (*compar) (const char *, const char *) =
+    int (*compar)(const char *, const char *) =
         (Scr->CaseSensitive ? strcmp : XmuCompareISOLatin1);
 
     if (!menu)
@@ -2281,21 +2281,33 @@ NeedToDefer(MenuRoot *root)
     return FALSE;
 }
 
+/*
+ * We cannot safely free a value passed to putenv, but we can cache the most
+ * recent value, reducing the memory leaks.
+ */
+#define cache_env(saved) \
+    if (saved == NULL || strcmp(saved, update)) { \
+        saved = update; \
+    } else { \
+        free(update); \
+        update = saved; \
+    }
+
 static void
 Execute(const char *s)
 {
-    /* FIXME: is all this stuff needed?  There could be security problems here. */
-    static char buf[256];
+    static const char display_eqls[] = "DISPLAY=";
+    static char *main_display;
+    static char *exec_display;
+
     char *ds = DisplayString(dpy);
     char *colon, *dot1;
-    char oldDisplay[256];
-    char *doisplay;
-    int restorevar = 0;
+    char *oldDisplay = NULL;
+    char *value;
+    Bool restorevar = False;
 
-    oldDisplay[0] = '\0';
-    doisplay = getenv("DISPLAY");
-    if (doisplay)
-        strcpy(oldDisplay, doisplay);
+    value = getenv("DISPLAY");
+    oldDisplay = strdup(value ? value : "");
 
     /*
      * Build a display string using the current screen number, so that
@@ -2305,23 +2317,38 @@ Execute(const char *s)
      */
     colon = strrchr(ds, ':');
     if (colon) {                /* if host[:]:dpy */
-        strcpy(buf, "DISPLAY=");
-        strcat(buf, ds);
-        colon = buf + 8 + (colon - ds); /* use version in buf */
-        dot1 = strchr(colon, '.');      /* first period after colon */
-        if (!dot1)
-            dot1 = colon + strlen(colon);       /* if not there, append */
-        (void) sprintf(dot1, ".%d", Scr->screen);
-        putenv(buf);
-        restorevar = 1;
+        size_t need = sizeof(display_eqls) + strlen(ds) + 10;
+        char *update = malloc(need);
+
+        if (update != NULL) {
+            strcpy(update, display_eqls);
+            strcat(update, ds);
+            colon = strrchr(update, ':');
+            dot1 = strchr(colon, '.');  /* first period after colon */
+            if (dot1 == NULL)
+                dot1 = colon + strlen(colon);   /* if not there, append */
+            (void) sprintf(dot1, ".%d", Scr->screen);
+            cache_env(exec_display);
+            if (strcmp(update, oldDisplay)) {
+                putenv(update);
+                restorevar = True;
+            }
+        }
     }
 
     (void) system(s);
 
-    if (restorevar) {           /* why bother? */
-        (void) snprintf(buf, sizeof(buf), "DISPLAY=%s", oldDisplay);
-        putenv(buf);
+    if (restorevar) {
+        size_t need = sizeof(display_eqls) + strlen(oldDisplay);
+        char *update = malloc(need);
+
+        if (update != NULL) {
+            (void) snprintf(update, need, "%s%s", display_eqls, oldDisplay);
+            cache_env(main_display);
+            putenv(update);
+        }
     }
+    free(oldDisplay);
 }
 
 /**