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

[Pascal Cuoq]

Hello,

lines 34 and 35 in ucs4.h contain a signed arithmetic overflow:

http://git.savannah.gnu.org/cgit/libiconv.git/tree/lib/ucs4.h?id=1bd8e101afd5e2edf63ffcb10328ef4262b375e9#n34

The assignment in question is:

    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.

This may seem harmless, because an overwhelming majority of platforms use 2's complement, but compilers optimize on the basis that undefined behavior does not happen. See “The myth of signed arithmetic wraparound” for an example (http://www.kuliniewicz.org/blog/archives/2011/06/11/the-myth-of-signed-arithmetic-wraparound/ ). Here the undefinedness of shifting a value in the range 128-255 could, for instance, be used as excuse for an optimizing C compiler to assume that the truth value of the condition that follows is necessary 0:

if (wc == 0xfffe0000u) {

A fix is to convert the value to be shifted to a type wide enough to represent the shift by 24 of all unsigned char values:

((ucs4_t)s[0]) << 24

Bruno, would you be interested in more reports of the same kind, assuming the minor bugs such as this one are batched together to save both of us time? The minor bugs would be found as part of a deeper verification that also eliminates more serious bugs, for instance buffer overflows that could be caused by malicious inputs and have security consequences.

Pascal Cuoq

TrustInSoft

PS: A long time ago, while working on the analyzer used to find the issue above, I had to wrap my head around weak pointers, and a survey you had written was very helpful. This is as good a chance as any to express my deepest thanks for that.