bug-bash
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] printf %ls/%lc/%Q fixes


From: Grisha Levit
Subject: [PATCH] printf %ls/%lc/%Q fixes
Date: Thu, 19 Oct 2023 20:23:14 -0400

A few issues with handling of the new %ls, %lc and %Q conversion
specifications:

The %ls and %lc conversion specifications dereference a null pointer
if used without an argument:

    $ (printf %ls)
    Segmentation fault: 11
    $ (printf %lc)
    Segmentation fault: 11

The Q conversion specifier's precision handling applies when precision
is provided as part of the format string, but not when it comes form
an argument:

    $ printf '%.1Q\n' '**'
    \*
    $ printf '%.*Q\n' 1 '**'
    \*\*

There is code to handle integer overflow for precision values by capping
values to INT_MAX but it's broken for the %Q conversion specifier due to
reading the value into an int rather than an intmax_t prior to overflow
checks being applied:

    $ INT_MAX=$(getconf INT_MAX) OVERFLOW=$((INT_MAX*2 + 3))
    $ printf "%.${OVERFLOW}s" XY
    XY
    $ printf "%.${OVERFLOW}Q" XY
    X

A precision value of 0 is ignored for the %ls conversion specification:

    $ printf '[%.0s][%.0ls]' X Y
    [][Y]

A null argument to the %lc conversion specifier does not produce a null
byte. Per discussion in [1], Bash's behavior matches the standard as it
was written, but this is now considered a defect.

    $ printf '[%c][%lc]' '' '' | cat -v
    [^@][]

[1] https://austingroupbugs.net/view.php?id=1647
---
 builtins/printf.def | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/builtins/printf.def b/builtins/printf.def
index ae6c75c5..6d3fd9a4 100644
--- a/builtins/printf.def
+++ b/builtins/printf.def
@@ -252,12 +252,13 @@ static size_t conv_bufsize;
 static inline int
 decodeprec (char *ps)
 {
-  int mpr;
+  intmax_t mpr;

   mpr = *ps++ - '0';
   while (DIGIT (*ps))
     mpr = (mpr * 10) + (*ps++ - '0');
-  return mpr;
+  /* Error if precision > INT_MAX here? */
+  return (mpr < 0 || mpr > INT_MAX) ? INT_MAX : mpr;
 }

 int
@@ -490,7 +491,10 @@ printf_builtin (WORD_LIST *list)
                    ws[0] = wc;
                    ws[1] = L'\0';

-                   r = printwidestr (start, ws, 1, fieldwidth, precision);
+                   if (wc == L'\0')
+                     r = printstr (start, ws, 1, fieldwidth, precision);
+                   else
+                     r = printwidestr (start, ws, 1, fieldwidth, precision);
                    if (r < 0)
                      PRETURN (EXECUTION_FAILURE);
                    break;
@@ -514,7 +518,7 @@ printf_builtin (WORD_LIST *list)

                    wp = getwidestr (&slen);
                    r = printwidestr (start, wp, slen, fieldwidth, precision);
-                   free (wp);
+                   FREE (wp);
                    if (r < 0)
                      PRETURN (EXECUTION_FAILURE);
                    break;
@@ -647,20 +651,19 @@ printf_builtin (WORD_LIST *list)
            case 'Q':
              {
                char *p, *xp;
-               int r, mpr;
+               int r;
                size_t slen;

                r = 0;
                p = getstr ();
                /* Decode precision and apply it to the unquoted string. */
-               if (convch == 'Q' && precstart)
+               if (convch == 'Q' && (have_precision || precstart))
                  {
-                   mpr = decodeprec (precstart);
-                   /* Error if precision > INT_MAX here? */
-                   precision = (mpr < 0 || mpr > INT_MAX) ? INT_MAX : mpr;
+                   if (precstart)
+                     precision = decodeprec (precstart);
                    slen = strlen (p);
                    /* printf precision works in bytes. */
-                   if (precision < slen)
+                   if (precision >= 0 && precision < slen)
                      p[precision] = '\0';
                  }
                if (p && *p == 0)       /* XXX - getstr never returns null */
@@ -1517,6 +1520,14 @@ getwidestr (size_t *lenp)
   wchar_t *ws;
   const char *mbs;
   size_t slen, mblength;
+
+  if (garglist == 0)
+    {
+      if (lenp)
+       *lenp = 0;
+      return NULL;
+    }
+
   DECLARE_MBSTATE;

   mbs = garglist->word->word;
@@ -1545,8 +1556,11 @@ getwidechar (void)
 {
   wchar_t wc;
   size_t slen, mblength;
-  DECLARE_MBSTATE;

+  if (garglist == 0)
+    return L'\0';
+
+  DECLARE_MBSTATE;
   wc = 0;
   mblength = mbrtowc (&wc, garglist->word->word, locale_mb_cur_max, &state);
   if (MB_INVALIDCH (mblength))
@@ -1571,7 +1585,7 @@ convwidestr (wchar_t *ws, int prec)

   ts = (const wchar_t *)ws;

-  if (prec > 0)
+  if (prec >= 0)
     {
       rsize = prec * MB_CUR_MAX;
       ret = (char *)xmalloc (rsize + 1);
-- 
2.42.0



reply via email to

[Prev in Thread] Current Thread [Next in Thread]