Compilers can remove your integer overflow check
Wei Liu (刘炜) of Tencent Security Xuanwu Lab discovered an interesting issue in libexif: the code checked for an integer overflow when handling a field in EXIF files, but that check was silently removed by the compiler.
You can find the diff for the fix here: https://android.googlesource.c… . The original code has this line:
if (e->size+sizeof(unsigned short) < e->size) break;
To see whether e->size is so large that it almost overflows, the programmer thought to add the size of a terminator (UTF+0000 which is an unsigned short), and then to check whether that total size is actually smaller than the original number, which means that an overflow must have occured.
Compilers are smart however, and they remove unnecessary code. Assuming that sizeof(unsigned short) is 2 (which is true on most platforms), the compiler sees a check for x+2 > x which it evaluates to a constant value false. Because something + 2 can never be smaller, right?
And if you put if(false) whatever(); in your code, the compiler is not going to bother creating binary code for whatever() — it will be ignored completely.
So what you end up with is code that has the check, and could pass a code review or audit. But the binary, the machine code that actually runs on people’s devices, does not have the check!
So what is the fix? The check above was replaced by this one (omitting, for brevity, the logging code that was also added):
if (e->size > UINT_MAX - sizeof(unsigned short)) break;
This takes the value at which an unsigned int overflows (UINT_MAX), takes 2 off the top, and checks that the size of the argument (e) doesn’t exceed that. The compiler will not be able to fully evaluate this check at compile-time. It will see (again, on most platforms) that UINT_MAX is 4294967295, and sizeof(unsigned short) is 2, and get stuck at if(e->size > 4294967293), leaving it with no choice but to create binary code to do a run-time check.
Bottom line? Update, because there’s a fix. And developers, stop trying to be clever and write down exactly what you mean. :)
More info: CVE-2020-0452