[ft-devel] [PATCH RFC] src/sfnt/ttsbit0.c: fix broken pointer overflow checks

[Xi Wang]

Many compilers such as gcc and clang optimize away pointer overflow
checks `p + n < p', because pointer overflow is undefined behavior.
Use a safe form `n > p_limit - p' instead.

Also avoid possible integer overflow issues, for example, using
`num_glyphs > ( p_limit - p ) / 2' rather than `num_glyphs * 2'
given a large `num_glyphs'.
---
To verify this, try the following simplified code.

void bar(void);
void foo(char *p, unsigned n)
{
    if (p + n < p)
        bar();
}

$ gcc -S -o - -O2 t.c
foo:
.LFB0:
        .cfi_startproc
        rep
        ret
        .cfi_endproc

The entire check is gone (gcc 4.7); clang produces a similar result.
---
 src/sfnt/ttsbit0.c |   20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/src/sfnt/ttsbit0.c b/src/sfnt/ttsbit0.c
index ffef10a..246b776 100644
--- a/src/sfnt/ttsbit0.c
+++ b/src/sfnt/ttsbit0.c
@@ -823,11 +823,11 @@
     image_offset = FT_NEXT_ULONG( p );
 
     /* overflow check */
-    if ( decoder->eblc_base + decoder->strike_index_array + image_offset <
-           decoder->eblc_base )
+    p = decoder->eblc_base + decoder->strike_index_array;
+    if ( image_offset > p_limit - p )
       goto Failure;
 
-    p = decoder->eblc_base + decoder->strike_index_array + image_offset;
+    p += image_offset;
     if ( p + 8 > p_limit )
       goto NoBitmap;
 
@@ -894,11 +894,8 @@
 
         num_glyphs = FT_NEXT_ULONG( p );
 
-        /* overflow check */
-        if ( p + ( num_glyphs + 1 ) * 4 < p )
-          goto Failure;
-
-        if ( p + ( num_glyphs + 1 ) * 4 > p_limit )
+        /* overflow check for p + ( num_glyphs + 1 ) * 4 */
+        if ( num_glyphs > ( p_limit - p ) / 4 - 1 )
           goto NoBitmap;
 
         for ( mm = 0; mm < num_glyphs; mm++ )
@@ -936,11 +933,8 @@
 
         num_glyphs = FT_NEXT_ULONG( p );
 
-        /* overflow check */
-        if ( p + 2 * num_glyphs < p )
-          goto Failure;
-
-        if ( p + 2 * num_glyphs > p_limit )
+        /* overflow check for p + 2 * num_glyphs */
+        if ( num_glyphs > ( p_limit - p ) / 2 )
           goto NoBitmap;
 
         for ( mm = 0; mm < num_glyphs; mm++ )
-- 
1.7.10.4