Skip to content

Commit

Permalink
Coverity parsepdf.c address various reports
Browse files Browse the repository at this point in the history
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 (#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 (#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 (#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 (#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 (#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 (#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 (#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 (#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.
  • Loading branch information
tshinnic committed Sep 13, 2014
1 parent 2825e27 commit a7eb113
Showing 1 changed file with 17 additions and 7 deletions.
24 changes: 17 additions & 7 deletions fontforge/parsepdf.c
Expand Up @@ -404,16 +404,16 @@ return( false );
}
return( true );
}
value = copy(pdf_getdictvalue(pc));
value = pdf_getdictvalue(pc);
if ( value==NULL || strcmp(value,"null")==0 )
free(key);
else {
if ( pc->pdfdict.next>=pc->pdfdict.cnt ) {
pc->pdfdict.keys = realloc(pc->pdfdict.keys,(pc->pdfdict.cnt+=20)*sizeof(char *));
pc->pdfdict.values = realloc(pc->pdfdict.values,pc->pdfdict.cnt*sizeof(char *));
}
pc->pdfdict.keys [pc->pdfdict.next] = key ;
pc->pdfdict.values[pc->pdfdict.next] = value;
pc->pdfdict.keys [pc->pdfdict.next] = key;
pc->pdfdict.values[pc->pdfdict.next] = copy(value);
++pc->pdfdict.next;
}
}
Expand Down Expand Up @@ -577,7 +577,7 @@ static int pdf_findfonts(struct pdfcontext *pc) {
sscanf(desc, "%d", &dnum);
if ( *pt=='/' || *pt=='(' )
++pt;
tpt = copy(pt);
tpt = copy(pt);

dnum = pdf_getdescendantfont( pc,dnum );
if ( dnum > 0 ) {
Expand All @@ -590,7 +590,9 @@ static int pdf_findfonts(struct pdfcontext *pc) {
/* they no longer look as CID-keyed fonts */
pc->cmap_from_cid[k] = 1;
k++;
}
} else {
free(tpt);
}
}
}
}
Expand Down Expand Up @@ -646,6 +648,8 @@ return( val );
if ( val<0 || val>=pc->ocnt || pc->objs[val]==-1 )
return( 0 );
here = ftell(pc->pdf);
if ( here < 0 )
return( 0 );
if ( !pdf_findobject(pc,val))
return( 0 );
pdf = pc->compressed ? pc->compressed : pc->pdf;
Expand Down Expand Up @@ -1737,14 +1741,16 @@ static void add_mapping(SplineFont *basesf, long *mappings, int *uvals, int nuni
free(sc->name);
sc->name = name;
sc->unicodeenc = UniFromName(name,sf->uni_interp,&custom);
} else {
free(name);
}
}

static void pdf_getcmap(struct pdfcontext *pc, SplineFont *basesf, int font_num) {
FILE *file;
int i, j, gid, start, end, uni, cur=0, nuni, nhex, nchars, lo, *uvals;
long *mappings;
char tok[200], *ccval, prevtok[200];
long *mappings = NULL;
char tok[200], *ccval, prevtok[200]="";
SplineFont *sf = basesf->subfontcnt > 0 ? basesf->subfonts[0] : basesf;

if ( !pdf_findobject(pc,pc->cmapobjs[font_num]) || !pdf_readdict(pc) )
Expand Down Expand Up @@ -1827,8 +1833,10 @@ return;
EncMapFree( sf->map );
sf->map = EncMapFromEncoding(sf,FindOrMakeEncoding("Original"));
}
free(mappings);
return;
fail:
free(mappings);
LogError( _("Syntax errors while parsing ToUnicode CMap") );
}

Expand Down Expand Up @@ -1968,6 +1976,8 @@ return( NULL );
FontDict *fd;
file = pdf_insertpfbsections(file,pc);
fd = _ReadPSFont(file);
if ( fd==NULL)
return( NULL );
sf = SplineFontFromPSFont(fd);
PSFontFree(fd);
} else if ( type==2 ) {
Expand Down

0 comments on commit a7eb113

Please sign in to comment.