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

PDF ToUnicode CMap is not fully support #4

Open
coolwanglu opened this issue Aug 12, 2012 · 2 comments
Open

PDF ToUnicode CMap is not fully support #4

coolwanglu opened this issue Aug 12, 2012 · 2 comments
Labels

Comments

@coolwanglu
Copy link
Contributor

A ToUnicode CMap inside a PDF file shows how to interpret the charcodes into Unicode values.

It may refer to a standard CMap, and add some modification.

Currently those modifications by begin/end bfchar/bfrange in ToUnicode CMaps as parsed and recognized.

However the reference part is not parsed.

In the end of pdf_getcmap in parsepdf.c, it's assumed that the CMap has been fully parsed, and all glyphs have been mapped to correct locations, which is not the case if the ToUnicode CMap only refer to a standard CMap.

What's more, the CMap parsing code in encoding.c is static, I wonder why.

khaledhosny added a commit that referenced this issue Aug 27, 2012
I don't know why it is crshing now (may be the resources?) as this code have
been like that since the dawn of time.

Traceback:

  0xb78289b0 in check_image_buffers (gdisp=0x84d5c78, neww=<optimized out>, newh=7, is_bitmap=0) at gimagexdraw.c:1705
  1705	    if ( width > gdisp->gg.iwidth || depth!=gdisp->gg.img->depth ) {
  (gdb) bt
  #0  0xb78289b0 in check_image_buffers (gdisp=0x84d5c78, neww=<optimized out>, newh=7, is_bitmap=0) at gimagexdraw.c:1705
  #1  0xb782b595 in gximage_to_ximage (image=0x8527ff0, src=0xbfffa0a0, gw=<optimized out>) at gimagexdraw.c:1778
  #2  0xb782e8c2 in _GXDraw_Image (_w=0x9bf4bb0, image=<optimized out>, src=0xbfffa0a0, x=8250, y=7) at gimagexdraw.c:1981
  #3  0xb7807a98 in GDrawDrawScaledImage (w=0x9bf4bb0, img=0x8527ff0, x=8250, y=7) at gdraw.c:488
  #4  0xb781b513 in GListMarkDraw (pixmap=0x9bf4bb0, x=8250, y=1, height=19, state=gs_enabled) at ggadgets.c:482
  #5  0xb788c493 in GMatrixEdit_SubExpose (gme=0x9bd9448, pixmap=0x9bf4bb0, event=0xbfffa400) at gmatrixedit.c:1691
  #6  0xb788c6b1 in matrixeditsub_e_h (gw=0x9bf4bb0, event=0xbfffa400) at gmatrixedit.c:1723
  #7  0xb78040da in _GWidget_Container_eh (gw=0x9bd9178, event=0xbfffa400) at gcontainer.c:269
  #8  0xb7878001 in dispatchEvent (gdisp=0x84d5c78, event=0xbfffa5ec) at gxdraw.c:3959
  #9  0xb7878162 in GXDrawProcessOneEvent (gdisp=0x84d5c78) at gxdraw.c:3991
  #10 0xb7808606 in GDrawProcessOneEvent (gdisp=0x84d5c78) at gdraw.c:748
  #11 0x080c77ff in ContextChainEdit (sf=0x9aa3c68, fpst=0x8ad982c, gfi=0x9afbfc0, newname=0x0, layer=1) at contextchain.c:3207
  #12 0x0818f14a in _LookupSubtableContents (sf=0x9aa3c68, sub=0x8a9d43c, sd=0x0, def_layer=1) at lookupui.c:5531
  #13 0x0813138d in LookupSubtableContents (gfi=0x9afbfc0, isgpos=0) at fontinfo.c:5941
  #14 0x08136a16 in LookupMouse (gfi=0x9afbfc0, isgpos=0, event=0xbfffe930) at fontinfo.c:7336
  #15 0x08136ccc in lookups_e_h (gw=0x9b98230, event=0xbfffe930, isgpos=0) at fontinfo.c:7380
  #16 0x08136d78 in gsublookups_e_h (gw=0x9b98230, event=0xbfffe930) at fontinfo.c:7399
  #17 0xb788f3b0 in drawable_e_h (pixmap=0x9b98230, event=0xbfffe930) at gdrawable.c:219
  #18 0xb78048a7 in _GWidget_Container_eh (gw=0x9b98230, event=0xbfffe930) at gcontainer.c:393
  #19 0xb7878001 in dispatchEvent (gdisp=0x84d5c78, event=0xbfffeb1c) at gxdraw.c:3959
  #20 0xb7878480 in GXDrawEventLoop (gd=0x84d5c78) at gxdraw.c:4058
  #21 0xb78086c1 in GDrawEventLoop (gdisp=0x84d5c78) at gdraw.c:766
  #22 0x0822457c in main (argc=1, argv=0xbffff134) at startui.c:1501
@coolwanglu
Copy link
Contributor Author

I guess it's not a main concern of fontforge, so I'm closing it.

@khaledhosny
Copy link
Contributor

It is still an issue that need to be addressed (even if no one addresses it immediately, that is the point of having an issue tracker), reopening.

@khaledhosny khaledhosny reopened this Sep 1, 2012
@JoesCat JoesCat mentioned this issue Feb 13, 2013
7 tasks
tshinnic added a commit to tshinnic/fontforge that referenced this issue Sep 13, 2014
A mix of minor fixes against Coverity report items.  All of these
are for code in fontforge/parsepdf.c

=== Routine pdf_loadfont()    line ~ 1933
CID 1226268 (fontforge#1 of 1): Dereference null return value (NULL_RETURNS)

Coverity reported that a returned pointer could be NULL, but that error
return value wasn't being handled by calling code in pdf_loadfont().

Unlikely to happen (tmpfile() failing?) but I checked that pdf_loadfont()
callers could handle error return of NULL, and saw example use just above
area, so added guard against return of NULL from _ReadPSFont().

Coverity report was
>  CID 1226268 (fontforge#1 of 1): Dereference null return value (NULL_RETURNS)
>  15. dereference: Dereferencing a pointer that might be null fd when calling SplineFontFromPSFont. [show details]
>      1956        fd = _ReadPSFont(file);

Tested against PDF containing type 1 font ("/FontFile") both with and
without forcing the returned value to NULL.

=== Routine pdf_getinteger()   line ~ 636

Coverity complained that the return value from ftell() was being used
with fseek() without first checking for the error return value -1.
Added an "if(here<0) return(0)" emulating the several other error
returns in the routine pdf_getinteger().

Coverity report was
>  CID 1083667 (fontforge#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>  11. negative_returns: here is passed to a parameter that cannot be negative.
>       648    fseek(pc->pdf,here,SEEK_SET);

Could not test definitively as no available PDF had data that passed
through this code path.

=== Routine pdf_getcmap()   line ~ 1749

Coverity complained that the string variable 'prevtok' was being used
before being initialized, which was very true. Later in the code variable
'tok' would be copied into it, but no initial value was set.  The
surrounding code made mistakes unlikely but...

Coverity report was
>  CID 1225176 (fontforge#2 of 2): Uninitialized scalar variable (UNINIT)
>  8. uninit_use_in_call: Using uninitialized element of array prevtok when calling sscanf.

Tested with PDF having CMap and verified 'prevtok' was uninitialized,
and initialized after code change.
>   char tok[200], *ccval, prevtok[200];
>   char tok[200], *ccval, prevtok[200]="";

Coverity complained that dynamic calloc() into 'mappings' was not being
released, which was true.

Coverity report was
>  CID 1083111 (fontforge#4-1 of 5): Resource leak (RESOURCE_LEAK)
>  50. leaked_storage: Variable mappings going out of scope leaks the storage it points to.

Tested with PDF having CMap.

=== Routine pdf_findfonts()   line ~ 556

Coverity complained about an allocation leak.  Code needed to make a copy
of a transient value 'pt' (the font name) as a following call would erase
the value. But then the copy was only used inside a conditional, and if
false then no one released the memory.

Coverity report was
>  CID 1083101 (fontforge#2 of 3): Resource leak (RESOURCE_LEAK)
>  73. leaked_storage: Variable tpt going out of scope leaks the storage it points to.

Tested the true path (and found I'd coded it wrong and fixed to this
version).  Haven't found a PDF passing through the false path.

=== Routine add_mapping()    line ~ 1687

Coverity complained about an allocation leak.  Code works very hard to
create a name of many parts, but then only uses that within a conditional.
The memory was not released if the conditional was false.

Coverity report was
>  CID 1083105 (fontforge#1-2 of 3): Resource leak (RESOURCE_LEAK)
>  13. leaked_storage: Variable name going out of scope leaks the storage it points to.

Tested the true path, but not able to find a PDF that tests the false path.

=== Routine pdf_readdict()   line ~ 385

Coverity complained about a memory leak.  A copy of a string was being
made to save in a data structure, but that assignment was inside a
conditional.  The memory would be lost if the conditional went the
other way.

Coverity report was
>  CID 1083585 (fontforge#1 of 2): Resource leak (RESOURCE_LEAK)
>  16. leaked_storage: Variable value going out of scope leaks the storage it points to.

Tested with various PDFs, as all would pass through this routine.
@ghost ghost mentioned this issue Jul 19, 2016
@ghost ghost mentioned this issue Sep 10, 2016
Omnikron13 pushed a commit to Omnikron13/fontforge that referenced this issue May 31, 2022
==10627==ERROR: AddressSanitizer: global-buffer-overflow on address 0x00010e2239c1 at pc 0x000111258c3d bp 0x7ffee286c210 sp 0x7ffee286b988
WRITE of size 4 at 0x00010e2239c1 thread T0
    #0 0x111258c3c in scanf_common(void*, int, bool, char const*, __va_list_tag*) (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x27c3c)
    #1 0x111258d6d in wrap_vsscanf (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x27d6d)
    #2 0x11125902c in wrap_sscanf (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x2802c)
    fontforge#3 0x10de70b21 in PrefsUI_LoadPrefs prefs.c:1230
    fontforge#4 0x10e02e0ce in fontforge_main startui.c:1109
    fontforge#5 0x10d654b11 in main main.c:33
    fontforge#6 0x7fff62d7b3d4 in start (libdyld.dylib:x86_64+0x163d4)

0x00010e2239c1 is located 63 bytes to the left of global variable 'fvhintingneededcol' defined in '../fontforgeexe/fontview.c:123:14' (0x10e223a00) of size 4
0x00010e2239c1 is located 0 bytes to the right of global variable 'warn_script_unsaved' defined in '../fontforgeexe/fontview.c:83:6' (0x10e2239c0) of size 1
SUMMARY: AddressSanitizer: global-buffer-overflow (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x27c3c) in scanf_common(void*, int, bool, char const*, __va_list_tag*)

warn_script_unsaved is declared as bool, but prefs.c:1230 casts its
pointer to int *, leading the issue above. Prefs of type pr_bool should
be int as well, FontForge is pre-C99 and does not know bool.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants