pgxml bug (crash) in xslt_proc.c

Mark Simonetti <marks@opalsoftware.co.uk> writes:

I think I've found a bug in xslt_proc.c in the pgxml contrib module that
I've also fixed. It is a serious bug as it crashes the entire database
backend. Do I just describe it here, or is there somewhere else to
report that, or is there a way for me to submit the actual bug fix?

If you want you can report it to pgsql-security at postgresql.org
instead of the regular bugs list. We'd probably only treat it as
a security issue if the bug seems exploitable for more than a mere
crash (eg, if it could lead to privilege escalation or arbitrary
code execution). If you're not sure about the possible consequences
probably best to let the -security list see it first.

Whichever way you report it, please include your proposed fix along
with the bug report.

regards, tom lane

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Hi Tom,
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). 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.

The offending part seemed to be freeing the transform "xslt_ctxt"
context before freeing the doc "doctree". Indeed if I check the XSLT
example with the XSLT distribution
(http://xmlsoft.org/libxslt/downloads.html) they do free the transform
context first.

I'm guessing that when the transform context is freed, it is trying to
free something related to the previously already freed document.

I also switched the order in the "catch" segment just above (line 147
onwards).

Regards,
Mark.
--

On 11/10/2014 16:33, Tom Lane wrote:

Mark Simonetti <marks@opalsoftware.co.uk> writes:

I think I've found a bug in xslt_proc.c in the pgxml contrib module that
I've also fixed. It is a serious bug as it crashes the entire database
backend. Do I just describe it here, or is there somewhere else to
report that, or is there a way for me to submit the actual bug fix?

If you want you can report it to pgsql-security at postgresql.org
instead of the regular bugs list. We'd probably only treat it as
a security issue if the bug seems exploitable for more than a mere
crash (eg, if it could lead to privilege escalation or arbitrary
code execution). If you're not sure about the possible consequences
probably best to let the -security list see it first.

Whichever way you report it, please include your proposed fix along
with the bug report.

regards, tom lane

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

PS, I forgot to mention that this is on a Windows system. I have not
yet tried to repeat it on Linux but I can give it a try.

Regards,
Mark.
--

On 11/10/2014 16:52, Mark Simonetti wrote:

Hi Tom,
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). 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.

The offending part seemed to be freeing the transform "xslt_ctxt"
context before freeing the doc "doctree". Indeed if I check the XSLT
example with the XSLT distribution
(http://xmlsoft.org/libxslt/downloads.html) they do free the transform
context first.

I'm guessing that when the transform context is freed, it is trying to
free something related to the previously already freed document.

I also switched the order in the "catch" segment just above (line 147
onwards).

Regards,
Mark.
--

On 11/10/2014 16:33, Tom Lane wrote:

Mark Simonetti <marks@opalsoftware.co.uk> writes:

I think I've found a bug in xslt_proc.c in the pgxml contrib module
that
I've also fixed. It is a serious bug as it crashes the entire database
backend. Do I just describe it here, or is there somewhere else to
report that, or is there a way for me to submit the actual bug fix?

If you want you can report it to pgsql-security at postgresql.org
instead of the regular bugs list. We'd probably only treat it as
a security issue if the bug seems exploitable for more than a mere
crash (eg, if it could lead to privilege escalation or arbitrary
code execution). If you're not sure about the possible consequences
probably best to let the -security list see it first.

Whichever way you report it, please include your proposed fix along
with the bug report.

regards, tom lane

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

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

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

On 11/10/2014 17:39, Tom Lane wrote:

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 ...

No problem, I will sort something out though it might be tomorrow now.

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.

Yes I thought the same once I found where the problem was it seemed
fairly trivial; finding it wasn't quite so straightforward admittedly!
I'm almost certain it is doctree; xslt_sec_prefs is not touched in the
transformation context free function and stylesheet is never associated
with it (though I suppose it could be indirectly). I'm just glad I
found it and can use this build of PostgreSQL for now as the people I'm
working for are a rather large client and they want to see it working.
This is the great thing about open source though.

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?

Yep no problem, I will try that tomorrow.

Regards,
Mark.
--

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs