[PATCH] Account for grouping in printf width (bug 30068)

[PATCH] Account for grouping in printf width (bug 30068)

[Carlos O'Donell]

This is a partial fix for mishandling of grouping when formatting
integers.  It properly computes the width in the presence of grouping
characteres when the with is larger than the number of significant
digits. The precision related issue is documented in bug 23432.

Co-authored-by: Andreas Schwab <schwab@suse.de>
---
 stdio-common/Makefile               |  2 ++
 stdio-common/tst-grouping3.c        | 54 +++++++++++++++++++++++++++++
 stdio-common/vfprintf-process-arg.c | 22 +++++++++---
 3 files changed, 73 insertions(+), 5 deletions(-)
 create mode 100644 stdio-common/tst-grouping3.c

diff --git a/stdio-common/Makefile b/stdio-common/Makefile
index 34fdd6d1f8..652d9e5f95 100644
--- a/stdio-common/Makefile
+++ b/stdio-common/Makefile
@@ -196,6 +196,7 @@ tests := \
   tst-gets \
   tst-grouping \
   tst-grouping2 \
+  tst-grouping3 \
   tst-long-dbl-fphex \
   tst-memstream-string \
   tst-obprintf \
@@ -340,6 +341,7 @@ $(objpfx)tst-sscanf.out: $(gen-locales)
 $(objpfx)tst-swprintf.out: $(gen-locales)
 $(objpfx)tst-vfprintf-mbs-prec.out: $(gen-locales)
 $(objpfx)tst-vfprintf-width-i18n.out: $(gen-locales)
+$(objpfx)tst-grouping3.out: $(gen-locales)
 endif
 
 tst-printf-bz18872-ENV = MALLOC_TRACE=$(objpfx)tst-printf-bz18872.mtrace \
diff --git a/stdio-common/tst-grouping3.c b/stdio-common/tst-grouping3.c
new file mode 100644
index 0000000000..e9e39218e2
--- /dev/null
+++ b/stdio-common/tst-grouping3.c
@@ -0,0 +1,54 @@
+/* Test printf with grouping and padding (bug 30068)
+   Copyright (C) 2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <locale.h>
+#include <stdio.h>
+#include <support/check.h>
+#include <support/support.h>
+
+static int
+do_test (void)
+{
+  char buf[80];
+
+  xsetlocale (LC_NUMERIC, "de_DE.UTF-8");
+
+  /* The format string has the following conversion specifier:
+     '  - Use thousands grouping.
+     +  - The result of a signed conversion shall begin with a sign.
+     -  - Left justified.
+     13 - Minimum 13 bytes of width.
+     9  - Minimum 9 digits of precision.
+
+     In bug 30068 the grouping characters were not accounted for in
+     the width, and were added after the fact resulting in a 15-byte
+     output instead of a 13-byte output.  The two additional bytes
+     come from the locale-specific thousands separator.  This increase
+     in size could result in a buffer overflow if a reasonable caller
+     calculated the size of the expected buffer using nl_langinfo to
+     determine the sie of THOUSEP in bytes.
+
+     This bug is distinct from bug 23432 which has to do with the
+     minimum precision calculation (digit based).  */
+  sprintf (buf, "%+-'13.9d", 1234567);
+  TEST_COMPARE_STRING (buf, "+001.234.567 ");
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/stdio-common/vfprintf-process-arg.c b/stdio-common/vfprintf-process-arg.c
index 24c9125f9f..8c0fcbcf78 100644
--- a/stdio-common/vfprintf-process-arg.c
+++ b/stdio-common/vfprintf-process-arg.c
@@ -186,11 +186,17 @@ LABEL (unsigned_number):      /* Unsigned number of base BASE.  */
   bool octal_marker = (prec <= number_length && number.word != 0
                        && alt && base == 8);
 
-  prec = MAX (0, prec - (workend - string));
+  /* At this point prec_inc is the additional bytes required for the
+     specificed precision.  It is 0 if the precision would not have
+     required additional bytes i.e. the number of input digits is more
+     than the precision.  It is greater than zero if the precision is
+     more than the number of digits without grouping (precision only
+     considers digits).  */
+  unsigned int prec_inc = MAX (0, prec - (workend - string));
 
   if (!left)
     {
-      width -= number_length + prec;
+      width -= number_length + prec_inc;
 
       if (number.word != 0 && alt && (base == 16 || base == 2))
         /* Account for 0X, 0x, 0B or 0b hex or binary marker.  */
@@ -221,7 +227,7 @@ LABEL (unsigned_number):      /* Unsigned number of base BASE.  */
           Xprintf_buffer_putc (buf, spec);
         }
 
-      width += prec;
+      width += prec_inc;
       Xprintf_buffer_pad (buf, L_('0'), width);
 
       if (octal_marker)
@@ -237,6 +243,8 @@ LABEL (unsigned_number):      /* Unsigned number of base BASE.  */
     }
   else
     {
+      /* Perform left justification adjustments.  */
+
       if (is_negative)
         {
           Xprintf_buffer_putc (buf, L_('-'));
@@ -263,9 +271,13 @@ LABEL (unsigned_number):      /* Unsigned number of base BASE.  */
       if (octal_marker)
 	--width;
 
-      width -= workend - string + prec;
+      /* Adjust the width by subtracting the number of bytes
+         required to represent the number with grouping characters
+	 (NUMBER_LENGTH) and any additional bytes required for
+	 precision.  */
+      width -= number_length + prec_inc;
 
-      Xprintf_buffer_pad (buf, L_('0'), prec);
+      Xprintf_buffer_pad (buf, L_('0'), prec_inc);
 
       if (octal_marker)
         Xprintf_buffer_putc (buf, L_('0'));
-- 
2.39.1

Re: [PATCH] Account for grouping in printf width (bug 30068)

[Andreas Schwab]

On Feb 04 2023, Carlos O'Donell wrote:

> This is a partial fix for mishandling of grouping when formatting
> integers.  It properly computes the width in the presence of grouping
> characteres when the with is larger than the number of significant
  characters           width
> digits. The precision related issue is documented in bug 23432.

Ok.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Account for grouping in printf width (bug 30068)

[Carlos O'Donell]

On 2/6/23 06:25, Andreas Schwab wrote:
> On Feb 04 2023, Carlos O'Donell wrote:
> 
>> This is a partial fix for mishandling of grouping when formatting
>> integers.  It properly computes the width in the presence of grouping
>> characteres when the with is larger than the number of significant
>   characters           width
>> digits. The precision related issue is documented in bug 23432.
> 
> Ok.
> 

Fixed. And pushed. Thank you!

I need to update NEWS and backport to release/2.37/master.

-- 
Cheers,
Carlos.