Tomas,


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

Can you provide a patch?

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.

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 ---