From a367c0640fcbf9872cde1f8c79f4f11658807ad1 Mon Sep 17 00:00:00 2001
From: Paul Smith <psmith@gnu.org>
Date: Sun, 26 Mar 2023 09:24:06 -0400
Subject: Ensure variable_buffer is always nul-terminated

* src/expand.c (variable_buffer_output): Allocate an extra byte and
set it to nul.  For safety, add assert() on PTR.
(variable_expand_string): Don't call variable_buffer_output just to
add a nul byte.
(allocated_variable_append): Ditto.
---
 src/expand.c   | 38 +++++++++++++++++++-------------------
 src/function.c |  6 ++++--
 src/variable.c |  2 +-
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/src/expand.c b/src/expand.c
index c7233e8c..d9d903fb 100644
--- a/src/expand.c
+++ b/src/expand.c
@@ -46,29 +46,33 @@ const floc **expanding_var = &reading_file;
 static size_t variable_buffer_length;
 char *variable_buffer;
 
-/* Subroutine of variable_expand and friends:
-   The text to add is LENGTH chars starting at STRING to the variable_buffer.
-   The text is added to the buffer at PTR, and the updated pointer into
-   the buffer is returned as the value.  Thus, the value returned by
-   each call to variable_buffer_output should be the first argument to
-   the following call.  */
+/* Append LENGTH chars of STRING at PTR which must point into variable_buffer.
+   The buffer will always be kept nul-terminated.
+   The updated pointer into the buffer is returned as the value.  Thus, the
+   value returned by each call to variable_buffer_output should be the first
+   argument to the following call.  */
 
 char *
 variable_buffer_output (char *ptr, const char *string, size_t length)
 {
   size_t newlen = length + (ptr - variable_buffer);
 
-  if ((newlen + VARIABLE_BUFFER_ZONE) > variable_buffer_length)
+  assert (ptr >= variable_buffer);
+  assert (ptr < variable_buffer + variable_buffer_length);
+
+  if (newlen + VARIABLE_BUFFER_ZONE + 1 > variable_buffer_length)
     {
       size_t offset = ptr - variable_buffer;
       variable_buffer_length = (newlen + 100 > 2 * variable_buffer_length
                                 ? newlen + 100
                                 : 2 * variable_buffer_length);
-      variable_buffer = xrealloc (variable_buffer, variable_buffer_length);
+      variable_buffer = xrealloc (variable_buffer, variable_buffer_length + 1);
       ptr = variable_buffer + offset;
     }
 
-  return mempcpy (ptr, string, length);
+  ptr = mempcpy (ptr, string, length);
+  *ptr = '\0';
+  return ptr;
 }
 
 /* Return a pointer to the beginning of the variable buffer.
@@ -83,9 +87,10 @@ initialize_variable_output ()
     {
       variable_buffer_length = 200;
       variable_buffer = xmalloc (variable_buffer_length);
-      variable_buffer[0] = '\0';
     }
 
+  variable_buffer[0] = '\0';
+
   return variable_buffer;
 }
 
@@ -229,10 +234,7 @@ variable_expand_string (char *line, const char *string, size_t length)
   line_offset = line - variable_buffer;
 
   if (length == 0)
-    {
-      variable_buffer_output (o, "", 1);
-      return variable_buffer;
-    }
+    return variable_buffer;
 
   /* We need a copy of STRING: due to eval, it's possible that it will get
      freed as we process it (it might be the value of a variable that's reset
@@ -426,7 +428,6 @@ variable_expand_string (char *line, const char *string, size_t length)
 
   free (save);
 
-  variable_buffer_output (o, "", 1);
   return (variable_buffer + line_offset);
 }
 
@@ -563,9 +564,8 @@ allocated_variable_append (const struct variable *v)
 
   variable_buffer = 0;
 
-  val = variable_append (v->name, strlen (v->name),
-                         current_variable_set_list, 1);
-  variable_buffer_output (val, "", 1);
+  variable_append (v->name, strlen (v->name), current_variable_set_list, 1);
+
   val = variable_buffer;
 
   variable_buffer = obuf;
@@ -604,7 +604,7 @@ install_variable_buffer (char **bufp, size_t *lenp)
   *bufp = variable_buffer;
   *lenp = variable_buffer_length;
 
-  variable_buffer = 0;
+  variable_buffer = NULL;
   initialize_variable_output ();
 }
 
diff --git a/src/function.c b/src/function.c
index 8b966484..62b13f52 100644
--- a/src/function.c
+++ b/src/function.c
@@ -2011,7 +2011,8 @@ static char *
 func_eq (char *o, char **argv, char *funcname UNUSED)
 {
   int result = ! strcmp (argv[0], argv[1]);
-  o = variable_buffer_output (o,  result ? "1" : "", result);
+  if (result)
+    o = variable_buffer_output (o,  "1", 1);
   return o;
 }
 
@@ -2026,7 +2027,8 @@ func_not (char *o, char **argv, char *funcname UNUSED)
   int result = 0;
   NEXT_TOKEN (s);
   result = ! (*s);
-  o = variable_buffer_output (o,  result ? "1" : "", result);
+  if (result)
+    o = variable_buffer_output (o,  "1", 1);
   return o;
 }
 #endif
diff --git a/src/variable.c b/src/variable.c
index cfb7dc03..acb3bcbb 100644
--- a/src/variable.c
+++ b/src/variable.c
@@ -1278,7 +1278,7 @@ shell_result (const char *p)
 
   args[0] = (char *) p;
   args[1] = NULL;
-  variable_buffer_output (func_shell_base (variable_buffer, args, 0), "\0", 1);
+  func_shell_base (variable_buffer, args, 0);
   result = strdup (variable_buffer);
 
   restore_variable_buffer (buf, len);
-- 
cgit v1.2.1