Consider the following C program: #include <stdio.h> #include <locale.h> int main (void) { if (setlocale (LC_ALL, "")) { printf ("1234567890123:\n"); printf ("%0+ -'13ld:\n", 1234567L); } return 0; } and try it with a locale that has a thousands separator, such as "LC_ALL=en_US.utf8". With glibc up to 2.36, I get as expected: 1234567890123: +1,234,567 : But I got a report from Klaus Dittrich (following his bug reports against GNU MPFR at https://sympa.inria.fr/sympa/arc/mpfr/2023-01/msg00001.html and https://sympa.inria.fr/sympa/arc/mpfr/2023-01/msg00017.html) that with glibc git on 2023-01-17, one gets: 1234567890123: +1,234,567 : i.e. padding is done to width 15 instead of 13. I've got another report that the MPFR test also fails with glibc 2.37, which is probably the above issue.
BTW, see also bug 23432 for other issues related to the thousands separator (but no regression like here).
I suppose that the same bug occurs with sprintf(), in which case it can yield a buffer overflow since additional characters are output. So I'm raising the importance.
I can confirm that mpfr (latest release + master) sprintf tests fail with glibc 2.37.
I suspect that the padding amount is computed ignoring the thousands separators, so that the increasing of the width would be equal to the total length of the thousands separator.
The patch for #23432 (https://sourceware.org/pipermail/libc-alpha/2023-January/144847.html) fixes the testcase.
Thanks for reporting this. Yes, this is a regression where after the refactor the implementation does not account for grouping characters during padding of the width. We'll work to get this fixed on the release branch so the distributions can pickup the fix directly. I'm marking this security plus since the width could be unexpected if you used a number of other APIs to pre-compute an expected with e.g. nl_langinfo, and digit counting. I've asked Andreas if he's working on a v2, because the test case needs a dependency on the test locale generation: https://sourceware.org/pipermail/libc-alpha/2023-February/145204.html
Confirmed that this could potentially cause a buffer overflow with sprintf, something like below. This will occur in the corner case where an application computes the size of buffer to be exactly enough to fit the digits in question, but sprintf ends up writing a couple of extra bytes, hence going beyond bounds. #include <stdio.h> #include <locale.h> #include <string.h> int main (void) { char buf[strlen ("1234567890123:") + 1]; __builtin_memset (buf, 'x', sizeof (buf)); if (setlocale (LC_ALL, "")) { printf ("1234567890123:\n"); printf ("%0+ -'13ld:\n", 1234567L); sprintf (buf, "%0+ -'13ld:", 1234567L); for (size_t i = 0; i < strlen ("1234567890123:") + 1; i++) { printf ("%c", buf[i]); } printf ("\n"); } return 0; }
Oops, hit send too soon. To finish, building with _FORTIFY_SOURCE should catch this problem immediately: e.g., build the above program with: $ gcc -D_FORTIFY_SOURCE=1 -O -o sprintf-test sprintf-test.c and run: $ LOCPATH=$PWD/localedata LC_ALL=en_US.UTF-8 ./elf/ld-linux-x86-64.so.2 --library-path .:./math:./elf:./dlfcn:./nss:./nis:./rt:./resolv:./mathvec:./support:./crypt:./nptl ../sprintf-test 1234567890123: +1,234,567 : *** buffer overflow detected ***: terminated Aborted (core dumped)
This is now fixed in master for glibc 2.38. I'm working to add a NEWS entry and then backport to release/2.37/master.
Fixed by c980549cc6a1c03c23cc2fe3e7b0fe626a0364b0 for glibc 2.38. Marking as required for 2.37.
Fixed in release/2.37/master with: commit 07b9521fc6369d000216b96562ff7c0ed32a16c4 Author: Carlos O'Donell <carlos@redhat.com> Date: Thu Jan 19 12:50:20 2023 +0100 Account for grouping in printf width (bug 30068) This is a partial fix for mishandling of grouping when formatting integers. It properly computes the width in the presence of grouping characters when the width 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> (cherry picked from commit c980549cc6a1c03c23cc2fe3e7b0fe626a0364b0)