global buffer overflow in ValidatePostScriptFontName (parsettf.c)

[fatal0]

# fontforge -lang=ff -c 'Open($1)' ./poc/ValidatePostScriptFontName-in-parsettf.c-global-buffer-overflow.otf
Copyright (c) 2000-2014 by George Williams. See AUTHORS for Contributors.
 License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
 with many parts BSD <http://fontforge.org/license.html>. Please read LICENSE.
 Based on sources from 09:14 UTC 13-Jun-2017-D.
 Based on source from git with hash: b8e5ff8f24955f4d7d59ac73c903cc088b21bdb6
Warning: Mac and Windows entries in the 'name' table differ for the
 SubFamily string in the language English (US)
 Mac String: Medium
Windows String: Regular
Warning: Mac and Windows entries in the 'name' table differ for the
 Fullname string in the language English (US)
 Mac String: CFF_Type-1_0x0d_expl Medium
Windows String: CFF_Type-1_0x0d_expl
=================================================================
==21454==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000001081a64 at pc 0x00000051b6be bp 0x7ffd652858e0 sp 0x7ffd652858d0
READ of size 4 at 0x000001081a64 thread T0
    #0 0x51b6bd in ValidatePostScriptFontName /root/afl_fuzz/project/test/fontforge/fontforge/parsettf.c:1459
    #1 0x526ab9 in readcfftopdict /root/afl_fuzz/project/test/fontforge/fontforge/parsettf.c:3006
    #2 0x52ae2a in readcfftopdicts /root/afl_fuzz/project/test/fontforge/fontforge/parsettf.c:3323
    #3 0x532df4 in readcffglyphs /root/afl_fuzz/project/test/fontforge/fontforge/parsettf.c:3934
    #4 0x54687c in readttf /root/afl_fuzz/project/test/fontforge/fontforge/parsettf.c:5527
    #5 0x552652 in _SFReadTTF /root/afl_fuzz/project/test/fontforge/fontforge/parsettf.c:6266
    #6 0x7e8756 in _ReadSplineFont /root/afl_fuzz/project/test/fontforge/fontforge/splinefont.c:1114
    #7 0x7ea217 in ReadSplineFont /root/afl_fuzz/project/test/fontforge/fontforge/splinefont.c:1285
    #8 0x7ea675 in LoadSplineFont /root/afl_fuzz/project/test/fontforge/fontforge/splinefont.c:1343
    #9 0x660df3 in bOpen /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:1854
    #10 0x6b967e in docall /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:9352
    #11 0x6b9f99 in handlename /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:9463
    #12 0x6be15f in term /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:9707
    #13 0x6bf96f in mul /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:9852
    #14 0x6bffcf in add /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:9897
    #15 0x6c0b1b in comp /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:9972
    #16 0x6c1338 in _and /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:10015
    #17 0x6c1871 in _or /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:10046
    #18 0x6c1e41 in assign /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:10078
    #19 0x6c2ff9 in expr /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:10156
    #20 0x6c4ab3 in ff_statement /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:10365
    #21 0x6c5b4f in ProcessNativeScript /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:10513
    #22 0x6c6536 in _CheckIsScript /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:10605
    #23 0x6c675b in CheckIsScript /root/afl_fuzz/project/test/fontforge/fontforge/scripting.c:10642
    #24 0x41fa8f in fontforge_main /root/afl_fuzz/project/test/fontforge/fontforgeexe/startnoui.c:113
    #25 0x41f705 in main /root/afl_fuzz/project/test/fontforge/fontforgeexe/main.c:39
    #26 0x7fc5cf78482f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #27 0x41f618 in _start (/root/afl_fuzz/project/test/fontforge/fontforgeexe/fontforge+0x41f618)

0x000001081a64 is located 35 bytes to the right of global variable 'ff_unicode_digitval' defined in 'utype.c:32789:21' (0x1071a40) of size 65537
0x000001081a64 is located 28 bytes to the left of global variable 'ff_unicode_utype' defined in 'utype.c:40984:14' (0x1081a80) of size 262148
SUMMARY: AddressSanitizer: global-buffer-overflow /root/afl_fuzz/project/test/fontforge/fontforge/parsettf.c:1459 ValidatePostScriptFontName
Shadow bytes around the buggy address:
  0x0000802082f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080208300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080208310: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080208320: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080208330: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x000080208340: 00 00 00 00 00 00 00 00 01 f9 f9 f9[f9]f9 f9 f9
  0x000080208350: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080208360: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080208370: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080208380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080208390: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
==21454==ABORTING

Testcase:
https://github.com/gnehsoah/poc/blob/master/fontforge/ValidatePostScriptFontName-in-parsettf.c-global-buffer-overflow.otf

[jtanx]

Hah, for once it's not (directly) FontForge... It looks like glibc is using a lut for isdigit, and when passed a negative input (char is signed??), asan flags it as a buffer overflow.

ahah... hah... ...or not... sigh... FontForge actually defines its own isdigit...

@JoesCat it looks like it's generated from makeutype, so it should probably be fixed there.

[JoesCat]

isdigit() is just one of the several defines that I think are better-off being made into functions that can return errors instead of simple mask-checks. If you look at the other defines, they can probably also be lumped into this same CVE in my opinion ...because someone can eventually find a creative way to break them too. ;-)
Similarly with #3096, if you look at "all" the functions in char.c, they too can use bullet-proofing even though just one function was listed/found ;-)
Both problems are in the Unicode directory I'd like to eventually tackle (but I need to get some non-FontForge stuff done and out of the way first ...maybe by late August???).
If you want to fix this and/or #3096, and/or more code that breaks makeutype.c, go ahead and break it. We fix makeutype later (it needs improving).

I can understand GW choosing this speedy-code-conserving method back in 200x when computers had less power and less internet access, especially for the size of FontForge which was basically one computer==one user, and did not have collaboration available then either.
Today's computers have more room, more speed, and internet access by default, so, knocking-back assumptions with bullet-proofed functions are the way to go these days.

[frank-trampe]

This neither crashes nor generates nasty Valgrind in current master. Are you still able to reproduce it?

[skef]

Closing as not reproducible.