Alex Rivera | Logout

Integer overflow not consistent

Asked 2012-08-20T03:55:52.777
8

Pardon me if this question has been posed before. I looked for answers to similar questions, but I'm still puzzled with my problem. So I will shoot the question anyway. I'm using a C library called libexif for image data. I run my application (which uses this library) both on my Linux desktop and my MIPS board. For a particular image file when I try to fetch the created time, I was getting an error/invalid value. On debugging further I saw that for this particular image file, I was not getting the tag (EXIF_TAG_DATE_TIME) as expected.

This library has several utility functions. Most functions are structured like below 

int16_t 
exif_get_sshort (const unsigned char *buf, ExifByteOrder order)
{
    if (!buf) return 0;
        switch (order) {
        case EXIF_BYTE_ORDER_MOTOROLA:
                return ((buf[0] << 8) | buf[1]);
        case EXIF_BYTE_ORDER_INTEL:
                return ((buf[1] << 8) | buf[0]);
        }

    /* Won't be reached */
    return (0);
}

uint16_t
exif_get_short (const unsigned char *buf, ExifByteOrder order)
{
    return (exif_get_sshort (buf, order) & 0xffff);
}

When the library tries to investigate the presence of tags in raw data, it calls exif_get_short() and assigns the value returned to a variable which is of type enum (int).

In the error case, exif_get_short() which is supposed to return unsigned value (34687) returns a negative number (-30871) which messes up the whole tag extraction from the image data.

34687 is outside the range of maximum representable int16_t value. And therefore leads to an overflow. When I make this slight modification in code, everything seems to work fine 

uint16_t
exif_get_short (const unsigned char *buf, ExifByteOrder order)
{
    int temp = (exif_get_sshort (buf, order) & 0xffff);
        return temp;
}
c integer-overflow libexif
Edit
Report

1 Answer

0

This is broken on so many levels, I don't know where to begin. Just look at what is done here:

  • The unsigned chars are read from the buffer.
  • They are assigned to a signed int16_t in exif_get_sshort.
  • This is assigned to an unsigned uint16_t in exif_get_short.
  • This is finally assigned to an enum which is of type signed int.

I'd say it's a miracle it works at all.

First, the assignment from the chars to int16_t is done with the values, not with the representation:

return ((buf[0] << 8) | buf[1]);

Which already throws you into the pit of undefined behaviour when the result actually is negative. Plus, it only works when the signed int representation of the implementation is the same as the one used in the file format (two's complement, I guess). It will fail for one's complement and sign-magnitude. So check what's the case for the MIPS implementation.

The clean way would be the other way around: Assign two chars from the buffer to an uint16_t, which will be well defined operations, and use this to return an int16_t. You could then care for further corrections of the value for different representations, if necessary.

Additionally, here:

if (!buf) return 0;

0 is a very bad choice of a return value, because it is a valid enum constant:

EXIF_TAG_GPS_VERSION_ID             = 0x0000,

If this is expected to be the default for invalidity, then the constant should be returned, not the magic number. Though this seems to be a generic function to return an int16_t, thus some other error mechanism should be used here.

For your specific question, well, follow the flow of conversions between sign

answered 2012-08-20T07:30:34.010

Your Answer