Re: Left-shift overflow in quotearg.c:151

[Bruno Haible via Gnulib discussion list]

Hi,

Kirill Furman wrote:
> Using gnulib and building it with UBSAN sanitizer
> I have found a bug in quotearg.c. Passing as char value,
> which modulus of 32 will be 31, const integer value will overflow.
> Here is PoC which causes this error:
> ...
> Here is the stacktrace:
> UBSAN_OPTIONS=print_stacktrace=1,halt_on_error=0 ./test
> quotearg.c:151:23: runtime error: left shift of 1 by 31 places
> cannot be represented in type 'int'
> #0 in set_char_quoting /home/as/gnulib/build/gllib/quotearg.c:151:23
> #1 in main /home/as/gnulib/build/test.c:10:9

Thanks for the reproducer and the stack trace.

Yes, (int)1 << 31 is undefined behaviour. An 'unsigned int' is the better
choice for doing bit mask and bit shifting operations.

> Solution - set const value 1 to unsigned const value:
> diff --git a/lib/quotearg.c b/lib/quotearg.c
> index a31ea3d100..358f836f77 100644
> --- a/lib/quotearg.c
> +++ b/lib/quotearg.c
> @@ -148,7 +148,7 @@ set_char_quoting (struct quoting_options *o, char c, 
> int i)
> (o ? o : &default_quoting_options)->quote_these_too + uc / INT_BITS;
> int shift = uc % INT_BITS;
> int r = (*p >> shift) & 1;
> - *p ^= ((i & 1) ^ r) << shift;
> + *p ^= ((i & 1U) ^ r) << shift;
> return r;
> }

Yes, this is a valid fix. Thanks. I prefer to change the type of r as well:


2025-03-10  Bruno Haible  <br...@clisp.org>

        quotearg: Avoid undefined behaviour.
        Reported by Kirill Furman <kfur...@astralinux.ru> in
        <https://lists.gnu.org/archive/html/bug-gnulib/2025-03/msg00037.html>.
        * lib/quotearg.c (set_char_quoting): Use 'unsigned int', not 'int', for
        doing bit mask operations.

diff --git a/lib/quotearg.c b/lib/quotearg.c
index a31ea3d100..a587cd5b45 100644
--- a/lib/quotearg.c
+++ b/lib/quotearg.c
@@ -147,8 +147,8 @@ set_char_quoting (struct quoting_options *o, char c, int i)
   unsigned int *p =
     (o ? o : &default_quoting_options)->quote_these_too + uc / INT_BITS;
   int shift = uc % INT_BITS;
-  int r = (*p >> shift) & 1;
-  *p ^= ((i & 1) ^ r) << shift;
+  unsigned int r = (*p >> shift) & 1;
+  *p ^= ((i & 1U) ^ r) << shift;
   return r;
 }