[poppler] RFC: Potential invalid pointer / release after free fix in HtmlOutputDev

[Reece Dunn]

NOTE: I haven't built this, as I cannot build at the moment (missing
fontconfig dependency in Ubuntu that I haven't figured out).

The original code is something like:

   GooString *str, *str1 = NULL;
   for(HtmlString *tmp=yxStrings;tmp;tmp=tmp->yxNext){
    if (tmp->htext){
       str=new GooString(tmp->htext);
       fprintf(f,"<text top=\"%d\" left=\"%d\"
",xoutRound(tmp->yMin),xoutRound(tmp->xMin));
       fprintf(f,"width=\"%d\" height=\"%d\"
",xoutRound(tmp->xMax-tmp->xMin),xoutRound(tmp->yMax-tmp->yMin));
       fprintf(f,"font=\"%d\">", tmp->fontpos);
      if (tmp->fontpos!=-1){
	str1=fonts->getCSStyle(tmp->fontpos, str);
      }
       fputs(str1->getCString(),f);
      delete str;
       delete str1;
       fputs("</text>\n",f);
     }

As can be seen, str1 is not initialised outside of the "if
(tmp->fontpos!=-1)" condition. This means that if tmp->fontpos == -1,
str1 is either NULL (if this is the first entry, resulting in a NULL
pointer dereference), or is pointing to the previous str1 object
already deleted (resulting in a delete-after-free), and before it a
NULL pointer access, or access to already released memory (with
potentially garbage data).

Thus, the code should be:

   GooString *str, *str1 = NULL;
   for(HtmlString *tmp=yxStrings;tmp;tmp=tmp->yxNext){
    if (tmp->htext){
       str=new GooString(tmp->htext);
       fprintf(f,"<text top=\"%d\" left=\"%d\"
",xoutRound(tmp->yMin),xoutRound(tmp->xMin));
       fprintf(f,"width=\"%d\" height=\"%d\"
",xoutRound(tmp->xMax-tmp->xMin),xoutRound(tmp->yMax-tmp->yMin));
       fprintf(f,"font=\"%d\">", tmp->fontpos);
      if (tmp->fontpos!=-1){
	str1=fonts->getCSStyle(tmp->fontpos, str);
       fputs(str1->getCString(),f);
       delete str1;
      }
      delete str;
       fputs("</text>\n",f);
     }

with str1 only being used and released inside that conditional.

Now, if tmp->fontpos == -1 the output is "<text></text>" as there is
no text being written. This does not make sense, so the
(tmp->fontpos!=-1) check can be moved to the (tmp->htext) check.

Thus my patch, which as I said is untested (hence the RFC).

NOTE: The "simple" html case is not outputting surrounding <text> or
<div> tags, so does not have this issue; it is only the "complex"
cases.

- Reece
-------------- next part --------------
A non-text attachment was scrubbed...
Name: invalid-pointer-delete-fix.diff
Type: text/x-patch
Size: 1746 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/poppler/attachments/20091104/e63c6aff/attachment-0001.bin