[RFC][Draft patch] Introduce IntegerSanitizer in GCC.

[Maxim Ostapenko]

CC'ing Jakub, Marek and Kostya, sanitizer maintainers in GCC.

On 04/07/16 14:12, Maxim Ostapenko wrote:
> Hi!
>
> Although in languages like C and C++ unsigned integer overflow 
> reliably wraps around and well defined, sometimes it may indicate an 
> error in code and lead to undesirable consequences and even security 
> vulnerabilities 
> (https://android-developers.blogspot.ru/2016/05/hardening-media-stack.html). 
> Clang has corresponding '-fsanitize=unsigned-integer-overflow' option 
> that may catch some of these errors (but misses some important cases 
> such as pointers overflow), GCC has not and instruments only signed 
> integer operations (S = S {+, -, *} S).
>
> Although GCC has a nice framework for instrumenting even "mixed" 
> binary operations (e.g. S = U + S) in middle end, implemented by Jakub 
> some time ago (see expand_addsub_overflow, expand_mul_overflow etc in 
> internal-fn.c), adding support for sanitizing unsigned and "mixed" 
> binary operations seems to be quite problematic in terms of huge 
> amount of false positive reports that may pop up in following cases:
>
> 1) Some code intentionally relies on overflows, e.g. when computing 
> hash codes. Unfortunately, it seems that we cannot do anything with it 
> on compiler side, programmer will need to do some filtering here by 
> himself.
>
> 2) Unfortunately, it seems that C and C++ FEs don't provide suitable 
> interfaces to get *LHS* real type when building binary expressions, so 
> *build_binary_op* doesn't know about *LHS* real type and we can easily 
> end up with false positive report e.g. in such code:
>
> int
> cmp (const char *s1, const char *s2)
> {
>   return strlen (s1) - strlen (s2);
> }
>
> because in gimple we would have something like this:
>
>   unsigned int tmp, len1, len2;
>   int result;
>   .........
>   tmp = len1 - len2;  <===== FP here if len1 < len2
>   result = (int) tmp;
>   return result;
>
> 3) GCC relies on unsigned integer overflows when constructing 
> POINTER_PLUS expressions. We need a special treatment for this case, 
> because it's quite common to have a negative offset from a given 
> pointer and we don't want have false reports here.
>
> This very draft patch (of course we would need more testcases, 
> changelog entry etc) adds support for 
> '-fsanitize=unsigned-integer-overflow' to GCC for PLUS, MINUS and MULT 
> operations, extending its functionality to support pointers overflow 
> and binary operations with different signess of theirs operands 
> ("mixed" binary operations, say, S = U + S). It simply replaces all +, 
> - and * by new ISAN_CHECK_{ADD, SUB, MUL} intrinsics in C and C++ FEs 
> to be lowered later during intrinsics expansion. For example, if we 
> have a + b expression, it would be replaced in FE by following call:
>
> a + b -> ISAN_CHECK_ADD (a, b, a_unsiged_p, b_unsigned_p, 
> result_unsigned_p)
>
> where *a_unsiged_p* is 1 if *a* is unsigned and 0 otherwise. The same 
> rule applies for *b_unsigned_p*. The *result_unsigned_p* value should 
> reflect result type signess, but as mentioned in 2), we don't know it 
> when building binary expression, so we can only guess it from 
> arguments types and maybe change it (in a very hackish way) later (see 
> *isan_maybe_change_ifn_sign_arg* function) in order to reduce number 
> of FPs (in fact, these hacks can significantly reduce FPs ratio, ~ 
> 30%). These values will be used in *expand_addsub_overflow* and 
> *expand_mul_overflow* functions to compute overflow correctly.
>
> This patch survives GCC bootstrap, I've also managed to build Firefox 
> and Chromium with '-fsanitize=unsigned-integer-overflow' enabled. The 
> tool found several bugs in opensource software:
>
> 1) Integer overflow when computing len parameter in "file" utility 
> (http://bugs.gw.com/view.php?id=555)
> 2) Pointer overflow (nullptr - 1) in Google's skia library 
> (https://bugs.chromium.org/p/skia/issues/detail?id=5415)
> 3) Unsigned integer overflow in HAL component in Firefox 
> (https://bugzilla.mozilla.org/show_bug.cgi?id=1280514)
>
> Is community interested in such a tool? Any feedback would be greatly 
> appreciated, especially some points how we can figure out real result 
> type for *LHS* of binary expressions in
> *build_binary_op* in order to avoid unnecessary FPs and ugly hacks in 
> FEs.
>
> Thanks,
> -Maxim

-------------- next part --------------
A non-text attachment was scrubbed...
Name: isan-upstream-1.diff
Type: text/x-diff
Size: 49134 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc/attachments/20160705/863641a5/attachment.bin>