Pádraig Brady wrote: > This seems to have missed some overflow cases > (which I noticed when auditing c32width() usage in coreutils/gnulib).
Indeed, yes, I missed these in 2011. > I'll push the attached later to address this. But the function also has another problem: When the count overflows before an unprintable character is encountered in the string, the function currently returns INT_MAX, where it should in fact return -1. The following patches fix that, making your proposed patch unnecessary. 2026-10-03 Bruno Haible <[email protected]> mbswidth: Fix overflow handling. * lib/mbswidth.c: Include <stdint.h>. (mbsnwidth): When the width accumulator becomes > INT_MAX before an unprintable character is seen, return -1, not INT_MAX. * modules/mbswidth (Depends-on): Add stdint-h, assert-h. 2026-10-03 Bruno Haible <[email protected]> wcswidth, c32swidth: Simplify. * lib/wcswidth.c: Include <stdint.h>. * lib/c32swidth.c: Likewise. * lib/wcswidth-impl.h (FUNC): Simplify by using a large-enough 'count' variable. * modules/wcswidth (Depends-on): Add stdint-h, assert-h. * modules/c32swidth (Depends-on): Likewise.
>From 7126bb5d6c4799e193379fe79e8aea50d0d3a60b Mon Sep 17 00:00:00 2001 From: Bruno Haible <[email protected]> Date: Sat, 3 Oct 2026 23:37:03 +0200 Subject: [PATCH 1/2] wcswidth, c32swidth: Simplify. * lib/wcswidth.c: Include <stdint.h>. * lib/c32swidth.c: Likewise. * lib/wcswidth-impl.h (FUNC): Simplify by using a large-enough 'count' variable. * modules/wcswidth (Depends-on): Add stdint-h, assert-h. * modules/c32swidth (Depends-on): Likewise. --- ChangeLog | 10 ++++++++++ lib/c32swidth.c | 1 + lib/wcswidth-impl.h | 36 +++++++++--------------------------- lib/wcswidth.c | 1 + modules/c32swidth | 2 ++ modules/wcswidth | 2 ++ 6 files changed, 25 insertions(+), 27 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7c30897e06..bb913fb7b9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2026-10-03 Bruno Haible <[email protected]> + + wcswidth, c32swidth: Simplify. + * lib/wcswidth.c: Include <stdint.h>. + * lib/c32swidth.c: Likewise. + * lib/wcswidth-impl.h (FUNC): Simplify by using a large-enough 'count' + variable. + * modules/wcswidth (Depends-on): Add stdint-h, assert-h. + * modules/c32swidth (Depends-on): Likewise. + 2026-10-02 Bruno Haible <[email protected]> tzsanitize: Add more comments. diff --git a/lib/c32swidth.c b/lib/c32swidth.c index 4750d5fbea..5d79b8f1e9 100644 --- a/lib/c32swidth.c +++ b/lib/c32swidth.c @@ -33,6 +33,7 @@ c32swidth (const char32_t *s, size_t n) #else +# include <stdint.h> # include <limits.h> # define FUNC c32swidth diff --git a/lib/wcswidth-impl.h b/lib/wcswidth-impl.h index 5aae53121c..e8d18ede9d 100644 --- a/lib/wcswidth-impl.h +++ b/lib/wcswidth-impl.h @@ -18,40 +18,22 @@ int FUNC (const UNIT *s, size_t n) { - { - int count = 0; - for (; n > 0; s++, n--) - { - UNIT c = *s; - if (c == (UNIT)'\0') - break; - { - int width = CHARACTER_WIDTH (c); - if (width < 0) - goto found_nonprinting; - if (width > INT_MAX - count) - goto overflow; - count += width; - } - } - return count; - } + /* Since S consists of at most PTRDIFF_MAX - 1 units, the value of COUNT + stays <= (PTRDIFF_MAX - 1) * 2 < SIZE_MAX. */ + static_assert ((size_t) (PTRDIFF_MAX - 1) * 2 < SIZE_MAX); - /* The total width has become > INT_MAX. - Continue searching for a non-printing wide character. */ + size_t count = 0; for (; n > 0; s++, n--) { UNIT c = *s; if (c == (UNIT)'\0') break; - { - int width = CHARACTER_WIDTH (c); - if (width < 0) - goto found_nonprinting; - } - overflow: ; + int width = CHARACTER_WIDTH (c); + if (width < 0) + goto found_nonprinting; + count += width; } - return INT_MAX; + return (count > INT_MAX ? INT_MAX : count); found_nonprinting: return -1; diff --git a/lib/wcswidth.c b/lib/wcswidth.c index a3e3f834b9..ac43980555 100644 --- a/lib/wcswidth.c +++ b/lib/wcswidth.c @@ -20,6 +20,7 @@ /* Specification. */ #include <wchar.h> +#include <stdint.h> #include <limits.h> #define FUNC wcswidth diff --git a/modules/c32swidth b/modules/c32swidth index 7c61ef4493..4fa815789c 100644 --- a/modules/c32swidth +++ b/modules/c32swidth @@ -12,6 +12,8 @@ m4/codeset.m4 Depends-on: uchar-h wchar-h +stdint-h +assert-h wcswidth c32width diff --git a/modules/wcswidth b/modules/wcswidth index 13891eb608..3572f41d98 100644 --- a/modules/wcswidth +++ b/modules/wcswidth @@ -9,6 +9,8 @@ m4/wcswidth.m4 Depends-on: wchar-h +stdint-h [test $HAVE_WCSWIDTH = 0 || test $REPLACE_WCSWIDTH = 1] +assert-h [test $HAVE_WCSWIDTH = 0 || test $REPLACE_WCSWIDTH = 1] wcwidth [test $HAVE_WCSWIDTH = 0 || test $REPLACE_WCSWIDTH = 1] configure.ac: -- 2.53.0
>From 92d4a0deb5f8f332d3ea800f8fb41d0b56b4169a Mon Sep 17 00:00:00 2001 From: Bruno Haible <[email protected]> Date: Sat, 3 Oct 2026 23:40:08 +0200 Subject: [PATCH 2/2] mbswidth: Fix overflow handling. * lib/mbswidth.c: Include <stdint.h>. (mbsnwidth): When the width accumulator becomes > INT_MAX before an unprintable character is seen, return -1, not INT_MAX. * modules/mbswidth (Depends-on): Add stdint-h, assert-h. --- ChangeLog | 8 +++++++ lib/mbswidth.c | 55 +++++++++++++++++++----------------------------- modules/mbswidth | 2 ++ 3 files changed, 32 insertions(+), 33 deletions(-) diff --git a/ChangeLog b/ChangeLog index bb913fb7b9..47c92a6f07 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2026-10-03 Bruno Haible <[email protected]> + + mbswidth: Fix overflow handling. + * lib/mbswidth.c: Include <stdint.h>. + (mbsnwidth): When the width accumulator becomes > INT_MAX before an + unprintable character is seen, return -1, not INT_MAX. + * modules/mbswidth (Depends-on): Add stdint-h, assert-h. + 2026-10-03 Bruno Haible <[email protected]> wcswidth, c32swidth: Simplify. diff --git a/lib/mbswidth.c b/lib/mbswidth.c index 6f490f4c80..5f809a23e3 100644 --- a/lib/mbswidth.c +++ b/lib/mbswidth.c @@ -35,6 +35,9 @@ /* Get char32_t, mbrtoc32(), c32iscntrl(), c32width(). */ #include <uchar.h> +/* Get PTRDIFF_MAX, SIZE_MAX. */ +#include <stdint.h> + /* Get INT_MAX. */ #include <limits.h> @@ -56,9 +59,13 @@ mbswidth (const char *string, int flags) int mbsnwidth (const char *string, size_t nbytes, int flags) { + /* Since STRING consists of at most PTRDIFF_MAX - 1 bytes, the value of WIDTH + stays <= (PTRDIFF_MAX - 1) * 2 < SIZE_MAX. */ + static_assert ((size_t) (PTRDIFF_MAX - 1) * 2 < SIZE_MAX); + const char *p = string; const char *plimit = p + nbytes; - int width = 0; + size_t width = 0; if (MB_CUR_MAX > 1) { while (p < plimit) @@ -135,21 +142,13 @@ mbsnwidth (const char *string, size_t nbytes, int flags) int w = c32width (wc); if (w >= 0) /* A printable multibyte character. */ - { - if (w > INT_MAX - width) - goto overflow; - width += w; - } + width += w; else /* An unprintable multibyte character. */ if (!(flags & MBSW_REJECT_UNPRINTABLE)) { if (!c32iscntrl (wc)) - { - if (width == INT_MAX) - goto overflow; - width++; - } + width++; } else return -1; @@ -163,33 +162,23 @@ mbsnwidth (const char *string, size_t nbytes, int flags) } break; } - return width; } - - while (p < plimit) + else { - unsigned char c = (unsigned char) *p++; - - if (isprint (c)) - { - if (width == INT_MAX) - goto overflow; - width++; - } - else if (!(flags & MBSW_REJECT_UNPRINTABLE)) + while (p < plimit) { - if (!iscntrl (c)) + unsigned char c = (unsigned char) *p++; + + if (isprint (c)) + width++; + else if (!(flags & MBSW_REJECT_UNPRINTABLE)) { - if (width == INT_MAX) - goto overflow; - width++; + if (!iscntrl (c)) + width++; } + else + return -1; } - else - return -1; } - return width; - - overflow: - return INT_MAX; + return (width > INT_MAX ? INT_MAX : width); } diff --git a/modules/mbswidth b/modules/mbswidth index 5bfdf9740c..afaa66444f 100644 --- a/modules/mbswidth +++ b/modules/mbswidth @@ -11,6 +11,8 @@ m4/mbswidth.m4 Depends-on: wchar-h uchar-h +stdint-h +assert-h mbrtoc32 mbsinit mbszero -- 2.53.0
