Re: pgxml bug (crash) in xslt_proc.c

Mark Simonetti <marks@opalsoftware.co.uk> writes:
> I hadn't really thought of it as a security issue, it came about from
> just trying to use it normally while developing software for one of my
> clients.  At first I found it hard to repeat, but I eventually found a
> query to repeat the problem 100% of the time. Unfortunately the XML I
> used to repeat it is vast and generated from lots of database data so it
> would be hard to submit that as a test case (though I can if it would
> help by capturing the XML data into a file and sending it along with the
> XSLT file).

Well, it would be nice to have a test case ...

> It seems to be to do with the order in which resources are
> freed:

> I changed this (xslt_proc.c, pgxml, postgres 9.3.5, line 167 onwards) : -

>      xsltFreeStylesheet(stylesheet);
>      xmlFreeDoc(restree);
>      xmlFreeDoc(doctree);
>      xsltFreeSecurityPrefs(xslt_sec_prefs);
>      xsltFreeTransformContext(xslt_ctxt);  <== crash here

> To this:

>      xsltFreeTransformContext(xslt_ctxt);
>      xsltFreeSecurityPrefs(xslt_sec_prefs);
>      xsltFreeStylesheet(stylesheet);
>      xmlFreeDoc(restree);
>      xmlFreeDoc(doctree);

> No more crash.

... but this seems like a pretty straightforward change: probably the
problem is that the xslt_ctxt has a dangling pointer to the
xslt_sec_prefs, stylesheet, or doctree.

Actually it seems to me the most sensible thing would be to free these
various objects in reverse order of creation, which would mean that it
ought to be

      xmlFreeDoc(restree);
      xsltFreeTransformContext(xslt_ctxt);
      xsltFreeSecurityPrefs(xslt_sec_prefs);
      xsltFreeStylesheet(stylesheet);
      xmlFreeDoc(doctree);

Would you try that on your test case and see if it's OK?

            regards, tom lane