From 6bad46b83f310668c8005b10ea594bcdb5ba5a5d Mon Sep 17 00:00:00 2001
From: Grisha Levit <grishalevit@gmail.com>
Date: Sat, 21 Oct 2023 21:08:07 -0400
Subject: [PATCH] printf: more error handling

The size of the buffer used for printf -v is tracked in an int but this
can overflow since the buffer can be built up by multiple vsnprintf(3)
calls, each of which can append up to INT_MAX bytes to the buffer:

    $ INT_MAX=$(getconf INT_MAX)
    $ printf -v VAR "%$((INT_MAX-1))s%$((INT_MAX-1))s"
    Bus error: 10

or when appending individual chars:

    $ printf -v VAR "%$((INT_MAX-1))sXXX"
    -bash: xrealloc: cannot allocate 18446744071562068032 bytes

The return value of vsnprintf(3) or printf(3) can be negative if, e.g.
the underlying write(2) call fails, or if a width or precision is out
of range.  Currently, this return value used unchecked as an offset
into vbuf:

    $ printf -v VAR "%.$((INT_MAX+1))s"
    heap-buffer-overflow builtins/printf.def:1253:15 in vbprintf

and added to the total when counting bytes written for the %n conversion
specifier:

    $ printf "%$((INT_MAX+1))s%n" "" N
    $ echo "$N"
    -1

Also, skip ferror(), fflush(), and clearerr() when running with the -v
flag.

- vblen: now size_t
- vbprintf: propagate negative vsnprintf() return value to caller
- PF: print error if vbprintf() has negative return value.  Use PRETURN
  instead of returning directly.
- PRETURN: if vflag != 0: don't check/flush stdout.  Don't skip buffer
  free-ing on shell variable binding failure.
- printf_builtin: remove redundant initialization of tw and empty format
  string check
- printf_builtin,printstr,printwidestr: if vflag != 0 skip ferror check
---
 builtins/printf.def | 122 +++++++++++++++++++++++---------------------
 1 file changed, 64 insertions(+), 58 deletions(-)

diff --git a/builtins/printf.def b/builtins/printf.def
index 5e84661d..c9b31912 100644
--- a/builtins/printf.def
+++ b/builtins/printf.def
@@ -105,6 +105,50 @@ $END
 extern int errno;
 #endif
 
+/* We free the buffer used by mklong() if it's `too big'. */
+#define PRETURN(value) \
+  do \
+    { \
+      QUIT; \
+      retval = value; \
+      if (conv_bufsize > 4096) \
+	{ \
+	  free (conv_buf); \
+	  conv_bufsize = 0; \
+	  conv_buf = 0; \
+	} \
+      if (vflag) \
+	{ \
+	  SHELL_VAR *v; \
+	  v = builtin_bind_variable (vname, vbuf, bindflags); \
+	  stupidly_hack_special_variables (vname); \
+	  if (v == 0 || readonly_p (v) || noassign_p (v)) \
+	    retval = EXECUTION_FAILURE; \
+	  if (vbsize > 4096) \
+	    { \
+	      free (vbuf); \
+	      vbsize = 0; \
+	      vbuf = 0; \
+	    } \
+	  else if (vbuf) \
+	    vbuf[0] = 0; \
+	} \
+      else \
+	{ \
+	  if (ferror (stdout) == 0) \
+	    fflush (stdout); \
+	  QUIT; \
+	  if (ferror (stdout)) \
+	    { \
+	      sh_wrerror (); \
+	      clearerr (stdout); \
+	      retval = EXECUTION_FAILURE; \
+	   } \
+        } \
+      return (retval); \
+    } \
+  while (0)
+
 #define PC(c) \
   do { \
     char b[2]; \
@@ -120,7 +164,9 @@ extern int errno;
 #define PF(f, func) \
   do { \
     int nw; \
-    clearerr (stdout); \
+    if (vflag == 0) \
+      clearerr (stdout); \
+    errno = 0; \
     if (have_fieldwidth && have_precision) \
       nw = vflag ? vbprintf (f, fieldwidth, precision, func) : printf (f, fieldwidth, precision, func); \
     else if (have_fieldwidth) \
@@ -129,56 +175,16 @@ extern int errno;
       nw = vflag ? vbprintf (f, precision, func) : printf (f, precision, func); \
     else \
       nw = vflag ? vbprintf (f, func) : printf (f, func); \
-    tw += nw; \
-    QUIT; \
-    if (ferror (stdout)) \
+    if (nw < 0) \
       { \
-	sh_wrerror (); \
-	clearerr (stdout); \
-	return (EXECUTION_FAILURE); \
+	if (vflag) \
+	  builtin_error ("%s", strerror (errno)); \
+	PRETURN (EXECUTION_FAILURE); \
       } \
+    tw += nw; \
+    QUIT; \
   } while (0)
 
-/* We free the buffer used by mklong() if it's `too big'. */
-#define PRETURN(value) \
-  do \
-    { \
-      QUIT; \
-      if (vflag) \
-	{ \
-	  SHELL_VAR *v; \
-	  v = builtin_bind_variable  (vname, vbuf, bindflags); \
-	  stupidly_hack_special_variables (vname); \
-	  if (v == 0 || readonly_p (v) || noassign_p (v)) \
-	    return (EXECUTION_FAILURE); \
-	} \
-      if (conv_bufsize > 4096 ) \
-	{ \
-	  free (conv_buf); \
-	  conv_bufsize = 0; \
-	  conv_buf = 0; \
-	} \
-      if (vbsize > 4096) \
-	{ \
-	  free (vbuf); \
-	  vbsize = 0; \
-	  vbuf = 0; \
-	} \
-      else if (vbuf) \
-	vbuf[0] = 0; \
-      if (ferror (stdout) == 0) \
-	fflush (stdout); \
-      QUIT; \
-      if (ferror (stdout)) \
-	{ \
-	  sh_wrerror (); \
-	  clearerr (stdout); \
-	  return (EXECUTION_FAILURE); \
-	} \
-      return (value); \
-    } \
-  while (0)
-
 #define SKIP1 "#'-+ 0"
 #define LENMODS "hjlLtz"
 
@@ -242,7 +248,7 @@ static int vflag = 0;
 static int bindflags = 0;
 static char *vbuf, *vname;
 static size_t vbsize;
-static int vblen;
+static size_t vblen;
 
 static intmax_t tw;
 
@@ -329,19 +335,15 @@ printf_builtin (WORD_LIST *list)
       return ((v == 0 || readonly_p (v) || noassign_p (v)) ? EXECUTION_FAILURE : EXECUTION_SUCCESS);
     }
 
+  /* If the format string is empty after preprocessing, return immediately. */
   if (list->word->word == 0 || list->word->word[0] == '\0')
     return (EXECUTION_SUCCESS);
 
   format = list->word->word;
-  tw = 0;
   retval = EXECUTION_SUCCESS;
 
   garglist = orig_arglist = list->next;
 
-  /* If the format string is empty after preprocessing, return immediately. */
-  if (format == 0 || *format == 0)
-    return (EXECUTION_SUCCESS);
-
   mb_cur_max = MB_CUR_MAX;
 
   /* Basic algorithm is to scan the format string for conversion
@@ -793,7 +795,7 @@ printf_builtin (WORD_LIST *list)
 	  modstart[1] = nextch;
 	}
 
-      if (ferror (stdout))
+      if (vflag == 0 && ferror (stdout))
 	{
 	  /* PRETURN will print error message. */
 	  PRETURN (EXECUTION_FAILURE);
@@ -928,7 +930,7 @@ printstr (char *fmt, char *string, int len, int fieldwidth, int precision)
   for (; padlen < 0; padlen++)
     PC (' ');
 
-  return (ferror (stdout) ? -1 : 0);
+  return ((vflag == 0 && ferror (stdout)) ? -1 : 0);
 }
 
 #if defined (HANDLE_MULTIBYTE)
@@ -1035,7 +1037,7 @@ printwidestr (char *fmt, wchar_t *wstring, size_t len, int fieldwidth, int preci
     PC (' ');
 
   free (string);
-  return (ferror (stdout) ? -1 : 0);
+  return ((vflag == 0 && ferror (stdout)) ? -1 : 0);
 }
 #endif
   
@@ -1261,7 +1263,7 @@ vbadd (char *buf, int blen)
 
 #ifdef DEBUG
   if  (strlen (vbuf) != vblen)
-    internal_error  ("printf:vbadd: vblen (%d) != strlen (vbuf) (%d)", vblen, (int)strlen (vbuf));
+    internal_error  ("printf:vbadd: vblen (%zu) != strlen (vbuf) (%zu)", vblen, strlen (vbuf));
 #endif
 
   return vbuf;
@@ -1277,6 +1279,8 @@ vbprintf (const char *format, ...)
   va_start (args, format);
   blen = vsnprintf (vbuf + vblen, vbsize - vblen, format, args);
   va_end (args);
+  if (blen < 0)
+    return (blen);
 
   nlen = vblen + blen + 1;
   if (nlen >= vbsize)
@@ -1286,6 +1290,8 @@ vbprintf (const char *format, ...)
       va_start (args, format);
       blen = vsnprintf (vbuf + vblen, vbsize - vblen, format, args);
       va_end (args);
+      if (blen < 0)
+	return (blen);
     }
 
   vblen += blen;
@@ -1293,7 +1299,7 @@ vbprintf (const char *format, ...)
 
 #ifdef DEBUG
   if  (strlen (vbuf) != vblen)
-    internal_error  ("printf:vbprintf: vblen (%d) != strlen (vbuf) (%d)", vblen, (int)strlen (vbuf));
+    internal_error  ("printf:vbprintf: vblen (%zu) != strlen (vbuf) (%zu)", vblen, strlen (vbuf));
 #endif
   
   return (blen);
-- 
2.43.0

