Re: [bug-gnu-libiconv] Signed arithmetic overflow in ucs4_mbtowc

[Daiki Ueno]

Pascal Cuoq <address@hidden> writes:

>     ucs4_t wc = (state
>                   ? s[0] + (s[1] << 8) + (s[2] << 16) + (s[3] << 24)
>                   : (s[0] << 24) + (s[1] << 16) + (s[2] << 8) + s[3]);
>
> In s[3]<<24 and s[0]<<24, although s is a pointer to unsigned char,
> the value is promoted to (signed) int before the shift. Then, in the
> typical case where int is 32-bit wide, the shift can overflow, which
> is technically undefined behavior.

Thanks for the report.  Such a problem can now also be detected with
gcc's -fsanitize=undefined option:
http://developerblog.redhat.com/2014/10/16/gcc-undefined-behavior-sanitizer-ubsan/

I got a couple more in libiconv:

  ./viscii.h:127:56: runtime error: left shift of 1 by 31 places cannot be 
represented in type 'int'
  ./tcvn.h:220:56: runtime error: left shift of 1 by 31 places cannot be 
represented in type 'int'

though the above case was not detected while runnning the tests.

Related to this, I'm going to add the attached patch to libunistring.

Regards,
--
Daiki Ueno

>From bb41ff0b77820134bd47640ba93249dd4a1b01c4 Mon Sep 17 00:00:00 2001
From: Daiki Ueno <address@hidden>
Date: Sat, 24 Jan 2015 11:11:34 +0900
Subject: [PATCH] unictype: avoid undefined left-shift behavior

* lib/unictype/bidi_of.c (uc_bidi_class): Building libunistring with
gcc's -fsanitize=shift and running its tests triggered:
  unictype/bidi_of.c:43:60: runtime error: left shift of 40167 by 16 \
    places cannot be represented in type 'int'
Cast LHS to 'unsigned int' after integer promotion.
* lib/unictype/categ_of.c (lookup_withtable): Likewise.
* lib/unictype/joininggroup_of.c (uc_joining_group): Likewise.
---
 ChangeLog                      | 11 +++++++++++
 lib/unictype/bidi_of.c         |  2 +-
 lib/unictype/categ_of.c        |  2 +-
 lib/unictype/joininggroup_of.c |  2 +-
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e0c12d3..a57bdd0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2015-01-24  Daiki Ueno  <address@hidden>
+
+       unictype: avoid undefined left-shift behavior
+       * lib/unictype/bidi_of.c (uc_bidi_class): Building libunistring with
+       gcc's -fsanitize=shift and running its tests triggered:
+         unictype/bidi_of.c:43:60: runtime error: left shift of 40167 by 16 \
+           places cannot be represented in type 'int'
+       Cast LHS to 'unsigned int' after integer promotion.
+       * lib/unictype/categ_of.c (lookup_withtable): Likewise.
+       * lib/unictype/joininggroup_of.c (uc_joining_group): Likewise.
+
 2015-01-20  Daiki Ueno  <address@hidden>
 
        libunistring: bump version of unitypes dependants
diff --git a/lib/unictype/bidi_of.c b/lib/unictype/bidi_of.c
index aa27d2f..4548ef5 100644
--- a/lib/unictype/bidi_of.c
+++ b/lib/unictype/bidi_of.c
@@ -40,7 +40,7 @@ uc_bidi_class (ucs4_t uc)
               /* level3 contains 5-bit values, packed into 16-bit words.  */
               unsigned int lookup3 =
                 ((u_bidi_category.level3[index3>>4]
-                  | (u_bidi_category.level3[(index3>>4)+1] << 16))
+                  | ((unsigned int) u_bidi_category.level3[(index3>>4)+1] << 
16))
                  >> (index3 % 16))
                 & 0x1f;
 
diff --git a/lib/unictype/categ_of.c b/lib/unictype/categ_of.c
index 867dc09..33f6c6c 100644
--- a/lib/unictype/categ_of.c
+++ b/lib/unictype/categ_of.c
@@ -40,7 +40,7 @@ lookup_withtable (ucs4_t uc)
               /* level3 contains 5-bit values, packed into 16-bit words.  */
               unsigned int lookup3 =
                 ((u_category.level3[index3>>4]
-                  | (u_category.level3[(index3>>4)+1] << 16))
+                  | ((unsigned int) u_category.level3[(index3>>4)+1] << 16))
                  >> (index3 % 16))
                 & 0x1f;
 
diff --git a/lib/unictype/joininggroup_of.c b/lib/unictype/joininggroup_of.c
index 95845fb..1626231 100644
--- a/lib/unictype/joininggroup_of.c
+++ b/lib/unictype/joininggroup_of.c
@@ -40,7 +40,7 @@ uc_joining_group (ucs4_t uc)
               /* level3 contains 7-bit values, packed into 16-bit words.  */
               unsigned int lookup3 =
                 ((u_joining_group.level3[index3>>4]
-                  | (u_joining_group.level3[(index3>>4)+1] << 16))
+                  | ((unsigned int) u_joining_group.level3[(index3>>4)+1] << 
16))
                  >> (index3 % 16))
                 & 0x7f;
 
-- 
2.1.3