Skip to content

Select a font in a PDF file#1

Closed
coolwanglu wants to merge 2 commits intofontforge:masterfrom
coolwanglu:pull
Closed

Select a font in a PDF file#1
coolwanglu wants to merge 2 commits intofontforge:masterfrom
coolwanglu:pull

Conversation

@coolwanglu
Copy link
Copy Markdown
Contributor

Selecting font by name using the FILENAME(FONTNAME) scheme has been implemented in SFReadPdfFont but not _SFReadPdfFont

However the latter is the one used by script, python and UI, which is weird.

I just move the font selection part from SF... to _SF...

@khaledhosny
Copy link
Copy Markdown
Contributor

Can you elaporate on how to test this? I can get indices to work, something like fontforge 'file.pdf(0)' but it complains 0 is not in the file, and got a crash when using font name instead.

@coolwanglu
Copy link
Copy Markdown
Contributor Author

Font name should be used, like "a.pdf(BFRJLG+Calibri)", where the font name is obtained using pdffonts.

Can you send me a link of the pdf causing crash?

@khaledhosny
Copy link
Copy Markdown
Contributor

It turned out the crash was not related to your patch, selecting the same font in the GUI chooser caused a crash too, so applied. Selecting fonts by index works for TTC file, so if you can get this to work for PDF files would be even better to keep things consistent.

@coolwanglu
Copy link
Copy Markdown
Contributor Author

Yes, I've thought about selection by index.
But what if there is a font named with a number?
Cannot figure out the logic.

@khaledhosny
Copy link
Copy Markdown
Contributor

PostScript font names can't be a number (or even start with a number), but I'm not sure about TrueType fonts, so I think it is OK to leave it as it is for now.

khaledhosny added a commit that referenced this pull request 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
@JoesCat JoesCat mentioned this pull request Feb 13, 2013
7 tasks
@coolwanglu coolwanglu mentioned this pull request Mar 5, 2013
tshinnic added a commit to tshinnic/fontforge that referenced this pull request Sep 13, 2014
Coverity found that values read from external data sources were not
checked for 'goodness'.  More specifically the upper/lower bounds were
not being checked.

Since a somewhat similar scenario passing through this area crashed
FontForge with bogus values, it seems reasonable to satisfy Coverity
and protect ourselves from this (unlikely) danger.

The starting object number and count of objects is read into variables
'start' and 'num' and used to populate a dynamically allocated array.
Protect against trouble by requiring that start and num are positive
integers less than a (very high) upper bound.  Also that the sum of
those is less than that same bound.  (bound derived as highest observed
object number rounded up to power of 10 and again multipled by 10)

The inserted code protects against values from both fscanf() calls at
top and bottom of routine FindObjects().

The Coverity reports were:
>  CID 1083582: Untrusted value as argument (TAINTED_SCALAR) [select issue]
>  6. overflow: Multiply operation overflows on operands start + num + 1L and 8UL. Example value for operand: start + num + 1L = 4611686018427387904.
>  CID 1083582 (fontforge#1 of 7): Untrusted loop bound (TAINTED_SCALAR)
>  8. tainted_data: Using tainted variable start + num as a loop boundary.

Tested against PDF files with the 'xref' table, including the tutorial
file doc/html/fontforge-tutorial.pdf
tshinnic added a commit to tshinnic/fontforge that referenced this pull request 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.
adrientetar pushed a commit that referenced this pull request Dec 6, 2014
@davelab6 davelab6 mentioned this pull request Jun 12, 2015
@ghost ghost mentioned this pull request Sep 10, 2016
@serval2412 serval2412 mentioned this pull request Apr 3, 2019
10 tasks
@pnemade pnemade mentioned this pull request Feb 14, 2020
8 tasks
Omnikron13 referenced this pull request in Omnikron13/fontforge 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.
iorsh pushed a commit to iorsh/fontforge that referenced this pull request Dec 22, 2023
@clsn clsn mentioned this pull request Jan 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants