Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory corruption followed by a crash in Metrics View of compact CID-keyed font #3946

Open
ctrlcctrlv opened this issue Sep 21, 2019 · 4 comments

Comments

@ctrlcctrlv
Copy link
Member

ctrlcctrlv commented Sep 21, 2019

Video of this crash: https://twitter.com/HW_BEAT_THAT/status/1175290889133944832
Necessary file: U-OTF-ShinGoUpr-Heavy.otf

#0  0x0000155553958755 in raise () at /usr/lib/libc.so.6
#1  0x0000155553943851 in abort () at /usr/lib/libc.so.6
#2  0x000015555399aa38 in __libc_message () at /usr/lib/libc.so.6
#3  0x00001555539a125a in  () at /usr/lib/libc.so.6
#4  0x00001555539a508c in _int_realloc () at /usr/lib/libc.so.6
#5  0x00001555539a602f in realloc () at /usr/lib/libc.so.6
#6  0x0000155554e670ce in MapAddEncodingSlot (map=0x555555e03e90, gid=1047) at fontforge/encoding.c:2317
#7  0x0000155554e6714d in FVAddEncodingSlot (fv=0x555555e03d10, gid=1047) at fontforge/encoding.c:2328
#8  0x0000155554e6741c in MapAddEnc
    (sf=0x5555563bdce0, sc=0x555555eddb50, basemap=0x555555e03e90, map=0x555555e03e90, baseenc=1047, gid=1047, fv=0x555555e03d10)
    at fontforge/encoding.c:2370
#9  0x0000155554e67942 in SFAddGlyphAndEncode (sf=0x5555563bdce0, sc=0x555555eddb50, basemap=0x555555e03e90, baseenc=1047) at fontforge/encoding.c:2435
#10 0x00001555550661dc in _SFMakeChar (sf=0x5555563bdce0, map=0x555555e03e90, enc=1047) at fontforge/splinefont.c:223
#11 0x0000155555066309 in SFMakeChar (sf=0x5555563bdce0, map=0x555555e03e90, enc=1047) at fontforge/splinefont.c:243
#12 0x000055555576e42b in MVSCFromUnicode (mv=0x555556324ec0, sf=0x5555563bdce0, map=0x555555e03e90, ch=957, bdf=0x0) at fontforgeexe/metricsview.c:1615
#13 0x000055555576fc4c in MVTextChanged (mv=0x555556324ec0) at fontforgeexe/metricsview.c:1977
#14 0x0000555555770376 in MV_TextChanged (g=0x5555560040c0, e=0x7fffffffc650) at fontforgeexe/metricsview.c:2100
#15 0x00005555558af987 in GTextFieldChanged (gt=0x5555560040c0, src=-1) at gdraw/gtextfield.c:213
#16 0x00005555558b7919 in gtextfield_key (g=0x5555560040c0, event=0x7fffffffc930) at gdraw/gtextfield.c:1860
#17 0x000055555584c928 in _GWidget_TopLevel_Key (top=0x555555e97d80, ew=0x555555e97d80, event=0x7fffffffc930) at gdraw/gcontainer.c:518
#18 0x000055555584d33f in _GWidget_TopLevel_eh (gw=0x555555e97d80, event=0x7fffffffc930) at gdraw/gcontainer.c:682
#19 0x0000555555860524 in _GGDKDraw_CallEHChecked (gw=0x555555e97d80, event=0x7fffffffc930, eh=0x55555584ce8e <_GWidget_TopLevel_eh>)
    at gdraw/ggdkdraw.c:329
#20 0x0000555555862f47 in _GGDKDraw_DispatchEvent (event=0x555555c00330, data=0x555555c1aeb0) at gdraw/ggdkdraw.c:1232
#21 0x00001555541bc8b4 in  () at /usr/lib/libgdk-3.so.0
#22 0x0000155554169904 in  () at /usr/lib/libgdk-3.so.0
#23 0x0000155553b4d3ae in g_main_context_dispatch () at /usr/lib/libglib-2.0.so.0
#24 0x0000155553b4f1c1 in  () at /usr/lib/libglib-2.0.so.0
#25 0x0000155553b4f201 in g_main_context_iteration () at /usr/lib/libglib-2.0.so.0
#26 0x0000555555865c72 in GGDKDrawEventLoop (gdisp=0x555555c1aeb0) at gdraw/ggdkdraw.c:2190
#27 0x0000555555851779 in GDrawEventLoop (gdisp=0x555555c1aeb0) at gdraw/gdraw.c:789
#28 0x0000555555815b29 in fontforge_main (argc=2, argv=0x7fffffffdd68) at fontforgeexe/startui.c:1415
#29 0x00005555555aab29 in main (argc=2, argv=0x7fffffffdd68) at fontforgeexe/main.c:33

Reproduction steps:

  1. Open necessary file
  2. Switch to CID ShinGoUpr-Ultra-Proportional
  3. Encoding→Compact
  4. New Metrics Window
  5. Type (very important, do not paste) /numbersign/numbersign/numbersign
  6. Crash!

Alternate stack trace:

double free or corruption (out)

Program received signal SIGABRT, Aborted.
0x0000155553958755 in raise () from /usr/lib/libc.so.6
(gdb) bt
#0  0x0000155553958755 in raise () at /usr/lib/libc.so.6
#1  0x0000155553943851 in abort () at /usr/lib/libc.so.6
#2  0x000015555399aa38 in __libc_message () at /usr/lib/libc.so.6
#3  0x00001555539a125a in  () at /usr/lib/libc.so.6
#4  0x00001555539a2c50 in _int_free () at /usr/lib/libc.so.6
#5  0x0000155553ed7553 in pango_glyph_string_free () at /usr/lib/libpango-1.0.so.0
#6  0x0000155553ecc7cc in  () at /usr/lib/libpango-1.0.so.0
#7  0x0000155553b226a8 in g_slist_foreach () at /usr/lib/libglib-2.0.so.0
#8  0x0000155553ecc776 in pango_layout_line_unref () at /usr/lib/libpango-1.0.so.0
#9  0x0000155553ecc88c in  () at /usr/lib/libpango-1.0.so.0
#10 0x0000155553ece1b5 in pango_layout_set_text () at /usr/lib/libpango-1.0.so.0
#11 0x00005555558c7e0d in GGDKDrawDoText8
    (w=0x555556349680, x=10, y=32, text=0x55556a8819c0 "0 (0x0) U+00A0 \"space\" NO-BREAK SPACE", cnt=-1, col=5869254, drawit=tf_drawit, arg=0x7fffffffc000) at gdraw/ggdkcdraw.c:1254
#12 0x000055555585391d in GDrawDrawText8
    (gw=0x555556349680, x=10, y=32, text=0x55556a8819c0 "0 (0x0) U+00A0 \"space\" NO-BREAK SPACE", cnt=-1, col=5869254) at gdraw/gdrawtxt.c:125
#13 0x0000555555708e59 in FVDrawInfo (fv=0x555555e052f0, pixmap=0x555556349680, event=0x7fffffffc510) at fontforgeexe/fontview.c:6122
#14 0x000055555570c0de in fv_e_h (gw=0x555556349680, event=0x7fffffffc510) at fontforgeexe/fontview.c:6895
#15 0x000055555584b75f in _GWidget_Container_eh (gw=0x555556349680, event=0x7fffffffc510) at gdraw/gcontainer.c:281
#16 0x000055555584d81c in _GWidget_TopLevel_eh (gw=0x555556349680, event=0x7fffffffc510) at gdraw/gcontainer.c:747
#17 0x0000555555860524 in _GGDKDraw_CallEHChecked (gw=0x555556349680, event=0x7fffffffc510, eh=0x55555584ce8e <_GWidget_TopLevel_eh>)
    at gdraw/ggdkdraw.c:329
#18 0x0000555555862f47 in _GGDKDraw_DispatchEvent (event=0x7fffffffc5d0, data=0x555555c1ab30) at gdraw/ggdkdraw.c:1232
#19 0x00001555541bc8b4 in  () at /usr/lib/libgdk-3.so.0
#20 0x00001555541af142 in  () at /usr/lib/libgdk-3.so.0
#21 0x00001555541aeb2f in  () at /usr/lib/libgdk-3.so.0
#22 0x00001555541aecfd in  () at /usr/lib/libgdk-3.so.0
#23 0x0000155553c2bb4a in g_signal_emit_valist () at /usr/lib/libgobject-2.0.so.0
#24 0x0000155553c2c7f0 in g_signal_emit () at /usr/lib/libgobject-2.0.so.0
#25 0x00001555541b8ad0 in  () at /usr/lib/libgdk-3.so.0
#26 0x00001555541c735c in  () at /usr/lib/libgdk-3.so.0
#27 0x0000155553b4ca74 in  () at /usr/lib/libglib-2.0.so.0
#28 0x0000155553b4d27f in g_main_context_dispatch () at /usr/lib/libglib-2.0.so.0
#29 0x0000155553b4f1c1 in  () at /usr/lib/libglib-2.0.so.0
#30 0x0000155553b4f201 in g_main_context_iteration () at /usr/lib/libglib-2.0.so.0
#31 0x0000555555865c72 in GGDKDrawEventLoop (gdisp=0x555555c1ab30) at gdraw/ggdkdraw.c:2190
#32 0x0000555555851779 in GDrawEventLoop (gdisp=0x555555c1ab30) at gdraw/gdraw.c:789
--Type <RET> for more, q to quit, c to continue without paging--
#33 0x0000555555815b29 in fontforge_main (argc=2, argv=0x7fffffffdd68) at fontforgeexe/startui.c:1415
#34 0x00005555555aab29 in main (argc=2, argv=0x7fffffffdd68) at fontforgeexe/main.c:33
@ctrlcctrlv
Copy link
Member Author

This CID code is a huge mess. I can't figure out how any of it is supposed to even work. MapAddEnc doesn't even check if maps are compact. SFFindSlot checks sf->cidmaster!=NULL && !map->enc->is_compact (if the font is CID-keyed and not compact), but then doesn't check if the font is CID-keyed AND compact.

I don't think it's fruitful I spend any more time on debugging this. I think we should acknowledge in documentation how buggy this stuff is and recommend people use third-party tools, such as those by Adobe, to turn CID-keyed fonts into normal GID-keyed fonts, before editing with FontForge.

What do you think @frank-trampe ?

@ctrlcctrlv
Copy link
Member Author

ctrlcctrlv commented Sep 23, 2019

(Or, even recommend using CID→Flatten instead of trying to use CID-keyed fonts as they are in FontForge; I consider myself a quite experienced FontForge user and have gotten nothing but crashes from doing it the other way…)

Flattened:
2019-09-23-164214_3562x825_scrot

@jtanx
Copy link
Contributor

jtanx commented Apr 18, 2020

Asan stacktrace
=================================================================
==16458==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62100017a95c at pc 0x7fa8cae97bc5 bp 0x7ffc986e8000 sp 0x7ffc986e7ff0
WRITE of size 4 at 0x62100017a95c thread T0
    #0 0x7fa8cae97bc4 in MapAddEnc ../fontforge/encoding.c:2372
    #1 0x7fa8cae97f0e in SFAddGlyphAndEncode ../fontforge/encoding.c:2445
    #2 0x7fa8cb27cd78 in _SFMakeChar ../fontforge/splinefont.c:224
    #3 0x55f64a8ca0f6 in MVSCFromUnicode ../fontforgeexe/metricsview.c:1626
    #4 0x55f64a8ca0f6 in MVTextChanged ../fontforgeexe/metricsview.c:1988
    #5 0x55f64a8cadbc in MV_TextChanged ../fontforgeexe/metricsview.c:2111
    #6 0x55f64a8cafc4 in MV_TextChanged ../fontforgeexe/metricsview.c:2095
    #7 0x55f64aae895c in GTextFieldChanged ../gdraw/gtextfield.c:213
    #8 0x55f64aafe5f6 in gtextfield_key ../gdraw/gtextfield.c:1868
    #9 0x55f64aa3092f in _GWidget_TopLevel_Key ../gdraw/gcontainer.c:518
    #10 0x55f64aa35881 in _GWidget_TopLevel_eh ../gdraw/gcontainer.c:673
    #11 0x55f64aa5cc4e in _GGDKDraw_CallEHChecked ../gdraw/ggdkdraw.c:346
    #12 0x55f64aa5ec84 in _GGDKDraw_DispatchEvent ../gdraw/ggdkdraw.c:1249
    #13 0x7fa8c9d8d764  (/usr/lib/x86_64-linux-gnu/libgdk-3.so.0+0x37764)
    #14 0x7fa8c9dbdf91  (/usr/lib/x86_64-linux-gnu/libgdk-3.so.0+0x67f91)
    #15 0x7fa8c90c0416 in g_main_context_dispatch (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c416)
    #16 0x7fa8c90c064f  (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c64f)
    #17 0x7fa8c90c06db in g_main_context_iteration (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c6db)
    #18 0x55f64aa59b2c in GGDKDrawEventLoop ../gdraw/ggdkdraw.c:2216
    #19 0x55f64a9cfe6d in fontforge_main ../fontforgeexe/startui.c:1414
    #20 0x7fa8c8ca4b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #21 0x55f64a572bf9 in _start (/home/jeremy/ffstaging/build/bin/fontforge+0x160bf9)

0x62100017a95c is located 52 bytes to the right of 4136-byte region [0x621000179900,0x62100017a928)
allocated by thread T0 here:
    #0 0x7fa8cbe52b50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)
    #1 0x7fa8cae966aa in CompactEncMap ../fontforge/encoding.c:2137
    #2 0x7fa8caee88dd in FVCompact ../fontforge/fontviewbase.c:1538
    #3 0x55f64a7eb0f1 in FVMenuCompact ../fontforgeexe/fontview.c:4788
    #4 0x55f64aa9d7f0 in gmenu_mouse ../gdraw/gmenu.c:948
    #5 0x55f64aa9edd2 in gmenu_eh ../gdraw/gmenu.c:1389
    #6 0x55f64aa5cc4e in _GGDKDraw_CallEHChecked ../gdraw/ggdkdraw.c:346
    #7 0x55f64aa5ec84 in _GGDKDraw_DispatchEvent ../gdraw/ggdkdraw.c:1249
    #8 0x7fa8c9d8d764  (/usr/lib/x86_64-linux-gnu/libgdk-3.so.0+0x37764)

SUMMARY: AddressSanitizer: heap-buffer-overflow ../fontforge/encoding.c:2372 in MapAddEnc
Shadow bytes around the buggy address:
  0x0c42800274d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c42800274e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c42800274f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4280027500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4280027510: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c4280027520: 00 00 00 00 00 fa fa fa fa fa fa[fa]fa fa fa fa
  0x0c4280027530: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280027540: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280027550: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280027560: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280027570: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==16458==ABORTING
Unrelated ASan stacktrace
=================================================================
==14753==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000008 (pc 0x556d47c5b849 bp 0x7ffc7adb7490 sp 0x7ffc7adb7370 T0)
==14753==The signal is caused by a READ memory access.
==14753==Hint: address points to the zero page.
    #0 0x556d47c5b848 in MVTextChanged ../fontforgeexe/metricsview.c:1947
    #1 0x556d47c5cdbc in MV_TextChanged ../fontforgeexe/metricsview.c:2111
    #2 0x556d47c5cfc4 in MV_TextChanged ../fontforgeexe/metricsview.c:2095
    #3 0x556d47e7a95c in GTextFieldChanged ../gdraw/gtextfield.c:213
    #4 0x556d47e905f6 in gtextfield_key ../gdraw/gtextfield.c:1868
    #5 0x556d47dc292f in _GWidget_TopLevel_Key ../gdraw/gcontainer.c:518
    #6 0x556d47dc7881 in _GWidget_TopLevel_eh ../gdraw/gcontainer.c:673
    #7 0x556d47deec4e in _GGDKDraw_CallEHChecked ../gdraw/ggdkdraw.c:346
    #8 0x556d47df0c84 in _GGDKDraw_DispatchEvent ../gdraw/ggdkdraw.c:1249
    #9 0x7f57cb825764  (/usr/lib/x86_64-linux-gnu/libgdk-3.so.0+0x37764)
    #10 0x7f57cb855f91  (/usr/lib/x86_64-linux-gnu/libgdk-3.so.0+0x67f91)
    #11 0x7f57cab58416 in g_main_context_dispatch (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c416)
    #12 0x7f57cab5864f  (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c64f)
    #13 0x7f57cab586db in g_main_context_iteration (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c6db)
    #14 0x556d47debb2c in GGDKDrawEventLoop ../gdraw/ggdkdraw.c:2216
    #15 0x556d47d61e6d in fontforge_main ../fontforgeexe/startui.c:1414
    #16 0x7f57ca73cb96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #17 0x556d47904bf9 in _start (/home/jeremy/ffstaging/build/bin/fontforge+0x160bf9)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV ../fontforgeexe/metricsview.c:1947 in MVTextChanged

@jtanx
Copy link
Contributor

jtanx commented Apr 18, 2020

I guess somewhat related to #1661.

If I understand correctly, cid-keyed fonts already use the custom encoding under the hood. When compacting is turned on, it also uses the custom encoding. While there's that is_compact flag, it's actually useless, as it's never set to true - a side-effect of c0c60e7 which removed the compact encoding.

So what's happening here is that it is_compact is false even though it should be true, and then everything blows up. No real easy fix that I can see, maybe #1661 should be addressed first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants