bugThe FreeType Project - Bugs: bug #35656, Static out-of-bounds read in...

 
 

bug #35656: Static out-of-bounds read in _bdf_parse_glyphs

Submitted by:  Mateusz Jurczyk <j00ru>
Submitted on:  Tue 28 Feb 2012 10:04:14 AM UTC  
 
Severity:  3 - Normal Item Group:  Crash
Status:  Fixed Privacy:  Public
Assigned to:  Werner LEMBERG <wl> Open/Closed:  Closed
Planned Release:  2.4.9

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Tue 20 Mar 2012 02:04:10 PM UTC, comment #7: 

Do we need two identical macros in the same file?

Line 428:
#define sbitset( m, cc ) \
         ( m[(FT_Byte)(cc) >> 3]  & ( 1 << ( (cc) & 7 ) ) )

Line 845:
#define isdigok( m, d )  (m[(unsigned char)(d) >> 3] & ( 1 << ( (d) & 7 ) ) )

I do also like FT_Byte more than unsigned char.

Alexei Podtelezhnikov <podtelez>
Project Member
Tue 20 Mar 2012 06:23:45 AM UTC, comment #6: 

I've changed the code accordingly.  Thanks!

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Mon 19 Mar 2012 09:28:54 PM UTC, comment #5: 

My proposal was to change isdigok() to:

#define isdigok( m, d )  (m[(unsigned char)(d) >> 3] & ( 1 << ( (d) & 7 ) ) )

I'm also attaching the mentioned testing change, with which following is reported for nethack12B.bdf.asan.4.761 when running ftbench via valgrind:

==30428== Invalid read of size 1
==30428==    at 0x452E58: _bdf_parse_glyphs (bdflib.c:1777)
==30428==    by 0x450EEB: _bdf_readstream (bdflib.c:773)
==30428==    by 0x45484B: bdf_load_font (bdflib.c:2401)
==30428==    by 0x455BEE: BDF_Face_Init (bdfdrivr.c:367)
==30428==    by 0x407BE0: open_face (ftobjs.c:1153)
==30428==    by 0x409367: FT_Open_Face (ftobjs.c:2080)
==30428==    by 0x407CF4: FT_New_Face (ftobjs.c:1215)
==30428==    by 0x402080: get_face (ftbench.c:624)
==30428==    by 0x40235B: main (ftbench.c:752)
==30428==  Address 0x56871f1 is 15 bytes before a block of size 32 alloc'd
==30428==    at 0x4C2A4CD: malloc (vg_replace_malloc.c:236)
==30428==    by 0x454727: bdf_load_font (bdflib.c:2379)
==30428==    by 0x455BEE: BDF_Face_Init (bdfdrivr.c:367)
==30428==    by 0x407BE0: open_face (ftobjs.c:1153)
==30428==    by 0x409367: FT_Open_Face (ftobjs.c:2080)
==30428==    by 0x407CF4: FT_New_Face (ftobjs.c:1215)
==30428==    by 0x402080: get_face (ftbench.c:624)
==30428==    by 0x40235B: main (ftbench.c:752)
==30428==
==30428== Invalid read of size 1
==30428==    at 0x452CF8: _bdf_parse_glyphs (bdflib.c:1752)
==30428==    by 0x450EEB: _bdf_readstream (bdflib.c:773)
==30428==    by 0x45484B: bdf_load_font (bdflib.c:2401)
==30428==    by 0x455BEE: BDF_Face_Init (bdfdrivr.c:367)
==30428==    by 0x407BE0: open_face (ftobjs.c:1153)
==30428==    by 0x409367: FT_Open_Face (ftobjs.c:2080)
==30428==    by 0x407CF4: FT_New_Face (ftobjs.c:1215)
==30428==    by 0x402080: get_face (ftbench.c:624)
==30428==    by 0x40235B: main (ftbench.c:752)
==30428==  Address 0x56871f6 is 10 bytes before a block of size 32 alloc'd
==30428==    at 0x4C2A4CD: malloc (vg_replace_malloc.c:236)
==30428==    by 0x454727: bdf_load_font (bdflib.c:2379)
==30428==    by 0x455BEE: BDF_Face_Init (bdfdrivr.c:367)
==30428==    by 0x407BE0: open_face (ftobjs.c:1153)
==30428==    by 0x409367: FT_Open_Face (ftobjs.c:2080)
==30428==    by 0x407CF4: FT_New_Face (ftobjs.c:1215)
==30428==    by 0x402080: get_face (ftbench.c:624)
==30428==    by 0x40235B: main (ftbench.c:752)
==30428==
_bdf_parse_glyphs: [line 113] Invalid keyword.
couldn't load font resource

(file #25410)

Tomas Hoger <thoger>
Mon 19 Mar 2012 06:39:08 PM UTC, comment #4: 

Tomas,

it doesn't hurt to be on the safe side :-)

Can you provide a patch?

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Mon 19 Mar 2012 03:06:49 PM UTC, comment #3: 

Interesting... no, I am no longer able to reproduce the issue with the two old samples and an ASAN build.

Mateusz Jurczyk <j00ru>
Project Member
Mon 19 Mar 2012 02:13:28 PM UTC, comment #2: 

Mateusz, have you been able to test the fix for this issue using ASAN?  It seems the fix still has a problem.  As c and line[] are (signed) char and isdigok() has "m[(d) >> 3]", the fix should still allow (smaller) hdigits[] underflow.  To test that I did a quick hack of the code to use heap alloced hdigits[] instead of data segment and got valgrind to report those underflows on Mateusz's test cases.

Other isdigok() uses (_bdf_ato*) also pass in char, so those cases should also by affected by this.  Changing isdigok() to do explicit cast to unsigned char may be the easiest fix.

Tomas Hoger <thoger>
Thu 01 Mar 2012 08:34:15 PM UTC, comment #1: 

Hopefully, this is fixed in git now.  Please test.

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Tue 28 Feb 2012 10:04:14 AM UTC, original submission:  

Crash tested with ftbench (current git branch of freetype2 and ft2demos), compiled under Ubuntu amd64 with AddressSanitizer (http://code.google.com/p/address-sanitizer/).

The memory error is only detectable using ASAN, as the Valgrind memcheck does not seem to detect out-of-bounds accesses on static arrays. Although the log is not very clear, it seems that the a2i array is referenced using an invalid index. Since it has 128 elements and is indexed with a value originating from type "char", the only possibility seems to be that a negative index is being used.

A similar crash is caught by ASAN in line 1766 for attached sample nethack16.bdf.asan.4.762, where again the following reference to "a2i" is present:

a2i[(int)line[nibbles]]

All references to a2i[] where a potentially negative index might be used should probably be guarded with a boundary check before accessing the array.

Crashlog from ASAN:

--- cut ---
(gdb) r ~/tmp/freetype_run2/jiskan16-16.bdf.asan.49.633
The program being debugged has been started already.
Start it from the beginning? (y or n) y

Starting program: freetype2/ftbench jiskan16-16.bdf.asan.49.633
[Thread debugging using libthread_db enabled]

Breakpoint 1, __asan_report_error

306     if (AtomicInc(&num_calls) > 1) return;
(gdb) where
#0  __asan_report_error (pc=<optimized out>, bp=<optimized out>, sp=<optimized out>, addr=DWARF-2 expression error: DW_OP_reg operations must be used either alone or in conjuction with DW_OP_piece or DW_OP_bit_piece.
)
    at address_sanitizer/asan/asan_rtl.cc:306
#1  0x00000000005c2e27 in __asan_report_load1 (addr=Unhandled dwarf expression opcode 0xd5
) at address_sanitizer/asan/asan_rtl.cc:214
#2  0x00000000005741e9 in _bdf_parse_glyphs (line=Unhandled dwarf expression opcode 0xec
) at freetype2/freetype-2.4.8/src/bdf/bdflib.c:1744
#3  0x000000000056cb86 in _bdf_readstream (stream=<optimized out>, callback=<optimized out>,
    client_data=<optimized out>, lno=<optimized out>) at freetype2/freetype-2.4.8/src/bdf/bdflib.c:765
#4  0x000000000056ad29 in bdf_load_font (stream=<optimized out>, extmemory=<optimized out>, opts=<optimized out>,
    font=<optimized out>) at freetype2/freetype-2.4.8/src/bdf/bdflib.c:2367
#5  0x0000000000567d7d in BDF_Face_Init (stream=<optimized out>, bdfface=Unhandled dwarf expression opcode 0x0
)
    at freetype2/freetype-2.4.8/src/bdf/bdfdrivr.c:367
#6  0x0000000000491460 in open_face (driver=Unhandled dwarf expression opcode 0x0
) at freetype2/freetype-2.4.8/src/base/ftobjs.c:1153
#7  0x0000000000490a23 in FT_Open_Face (library=<optimized out>, args=Unhandled dwarf expression opcode 0xd7
)
    at freetype2/freetype-2.4.8/src/base/ftobjs.c:2080
#8  0x000000000048fd46 in FT_New_Face (library=<optimized out>, pathname=<optimized out>, face_index=<optimized out>,
    aface=<optimized out>) at freetype2/freetype-2.4.8/src/base/ftobjs.c:1215
#9  0x0000000000487407 in get_face (face=0x0) at freetype2/ft2demos-2.4.8/src/ftbench.c:624
#10 0x0000000000487b61 in main (argc=<optimized out>, argv=<optimized out>)
    at freetype2/ft2demos-2.4.8/src/ftbench.c:752
(gdb) l freetype2/freetype-2.4.8/src/bdf/bdflib.c:1744
1739        for ( i = 0; i < nibbles; i++ )
1740        {
1741          c = line[i];
1742          if ( !c )
1743            break;
1744          bp = (FT_Byte)( ( bp << 4 ) + a2i[c] );
1745          if ( i + 1 < nibbles && ( i & 1 ) )
1746            *++bp = 0;
1747        }
1748
(gdb) c
Continuing.
=================================================================
==18216== ERROR: AddressSanitizer global-buffer-overflow on address 0x0000006fa1c6 at pc 0x5741e9 bp 0x7fffffffd570 sp 0x7fffffffd568
READ of size 1 at 0x0000006fa1c6 thread T0
  #0 0x5741e9 _bdf_parse_glyphs
  #1 0x56cb86 _bdf_readstream
  #2 0x56ad29 bdf_load_font
  #3 0x567d7d BDF_Face_Init
  #4 0x491460 open_face
  #5 0x490a23 FT_Open_Face
  #6 0x48fd46 FT_New_Face
  #7 0x487407 get_face
  #8 0x487b61 main
  #9 0x7ffff73ccd5d __libc_start_main
0x0000006fa1c6 is located 2 bytes to the right of global variable '.str (freetype2/freetype-2.4.8/src/bdf/bdf.c)' (0x6fa1c0) of size 4
  '.str (freetype2/freetype-2.4.8/src/bdf/bdf.c)' is ascii string 'bdf'
==18216== ABORTING
Stats: 1M malloced (1M for red zones) by 1035 calls
Stats: 0M realloced by 5 calls
Stats: 0M freed by 33 calls
Stats: 0M really freed by 0 calls
Stats: 36M (9221 full pages) mmaped in 9 calls
  mmaps   by size class: 8:16383; 9:8191; 10:4095; 11:2047; 12:1024; 13:512; 15:128; 19:8; 20:4;
  mallocs by size class: 8:996; 9:16; 10:9; 11:6; 12:3; 13:1; 15:1; 19:2; 20:1;
  frees   by size class: 8:18; 9:3; 10:8; 11:2; 12:1; 13:1;
  rfrees  by size class:
Stats: malloc large: 3 small slow: 8
Shadow byte and word:
  0x1000000df438: 4
  0x1000000df438: 04 f9 f9 f9 f9 f9 f9 f9
More shadow bytes:
  0x1000000df418: 00 00 00 00 00 00 00 00
  0x1000000df420: 00 00 00 00 00 00 00 00
  0x1000000df428: 00 00 00 00 00 00 00 00
  0x1000000df430: f9 f9 f9 f9 00 00 00 00
=>0x1000000df438: 04 f9 f9 f9 f9 f9 f9 f9
  0x1000000df440: 00 f9 f9 f9 f9 f9 f9 f9
  0x1000000df448: 00 04 f9 f9 f9 f9 f9 f9
  0x1000000df450: 00 06 f9 f9 f9 f9 f9 f9
  0x1000000df458: 00 03 f9 f9 f9 f9 f9 f9
[Inferior 1 (process 18216) exited with code 0351]
(gdb)
--- cut ---

Mateusz Jurczyk <j00ru>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #25410:  isdigok-debug.diff added by thoger (2KiB - text/x-patch)
file #25201:  jiskan16-16.bdf.asan.49.633 added by j00ru (1MiB - application/octet-stream)
file #25202:  nethack12B.bdf.asan.4.761 added by j00ru (24KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by podtelez (Posted a comment)
  • -email is unavailable- added by thoger (Posted a comment)
  • -email is unavailable- added by wl (Posted a comment)
  • -email is unavailable- added by j00ru (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2012-03-19 thoger Attached File- => Added isdigok-debug.diff, #25410
    2012-03-01 wl StatusNone => Fixed
        PrivacyPrivate => Public
        Assigned toNone => wl
        Open/ClosedOpen => Closed
        Planned ReleaseNone => 2.4.9
    2012-02-28 j00ru Attached File- => Added jiskan16-16.bdf.asan.49.633, #25201
        Attached File- => Added nethack12B.bdf.asan.4.761, #25202

    Back to the top


    Powered by Savane 3.5