From ef7429bb4f1433726cc8fc4fe3d134d8a439fab1 Mon Sep 17 00:00:00 2001 From: Nick Wellnhofer Date: Tue, 26 Apr 2016 15:36:48 +0200 Subject: [PATCH] Fix error handling in Saxon extension functions The old code could lead to a NULL pointer dereference. - Set XPath error if saxon:expression can't compile an expression. - Check return value in saxon:eval. Add first tests for Saxon extension functions. Found with afl-fuzz and ASan. --- configure.in | 1 + libexslt/saxon.c | 8 +++--- tests/exslt/Makefile.am | 2 +- tests/exslt/saxon/Makefile.am | 48 +++++++++++++++++++++++++++++++++++ tests/exslt/saxon/eval.1.out | 4 +++ tests/exslt/saxon/eval.1.xml | 3 +++ tests/exslt/saxon/eval.1.xsl | 22 ++++++++++++++++ tests/exslt/saxon/eval.2.err | 7 +++++ tests/exslt/saxon/eval.2.out | 2 ++ tests/exslt/saxon/eval.2.xml | 1 + tests/exslt/saxon/eval.2.xsl | 16 ++++++++++++ tests/exslt/saxon/eval.3.err | 6 +++++ tests/exslt/saxon/eval.3.out | 2 ++ tests/exslt/saxon/eval.3.xml | 1 + tests/exslt/saxon/eval.3.xsl | 16 ++++++++++++ win32/runtests.py | 3 +++ 16 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 tests/exslt/saxon/Makefile.am create mode 100644 tests/exslt/saxon/eval.1.out create mode 100644 tests/exslt/saxon/eval.1.xml create mode 100644 tests/exslt/saxon/eval.1.xsl create mode 100644 tests/exslt/saxon/eval.2.err create mode 100644 tests/exslt/saxon/eval.2.out create mode 100644 tests/exslt/saxon/eval.2.xml create mode 100644 tests/exslt/saxon/eval.2.xsl create mode 100644 tests/exslt/saxon/eval.3.err create mode 100644 tests/exslt/saxon/eval.3.out create mode 100644 tests/exslt/saxon/eval.3.xml create mode 100644 tests/exslt/saxon/eval.3.xsl diff --git a/configure.in b/configure.in index fee676fe..7e03d117 100644 --- a/configure.in +++ b/configure.in @@ -716,6 +716,7 @@ tests/exslt/Makefile tests/exslt/common/Makefile tests/exslt/functions/Makefile tests/exslt/math/Makefile +tests/exslt/saxon/Makefile tests/exslt/sets/Makefile tests/exslt/strings/Makefile tests/exslt/date/Makefile diff --git a/libexslt/saxon.c b/libexslt/saxon.c index e92ba8d0..491b31bc 100644 --- a/libexslt/saxon.c +++ b/libexslt/saxon.c @@ -101,9 +101,7 @@ exsltSaxonExpressionFunction (xmlXPathParserContextPtr ctxt, int nargs) { ret = xmlXPathCompile(arg); if (ret == NULL) { xmlFree(arg); - xsltGenericError(xsltGenericErrorContext, - "{%s}:%s: argument is not an XPath expression\n", - ctxt->context->functionURI, ctxt->context->function); + xmlXPathSetError(ctxt, XPATH_EXPR_ERROR); return; } xmlHashAddEntry(hash, arg, (void *) ret); @@ -147,6 +145,10 @@ exsltSaxonEvalFunction (xmlXPathParserContextPtr ctxt, int nargs) { expr = (xmlXPathCompExprPtr) xmlXPathPopExternal(ctxt); ret = xmlXPathCompiledEval(expr, ctxt->context); + if (ret == NULL) { + xmlXPathSetError(ctxt, XPATH_EXPR_ERROR); + return; + } valuePush(ctxt, ret); } diff --git a/tests/exslt/Makefile.am b/tests/exslt/Makefile.am index f749efdc..aeb58c1d 100644 --- a/tests/exslt/Makefile.am +++ b/tests/exslt/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in -SUBDIRS=common functions math sets strings dynamic date $(CRYPTO_TESTDIR) +SUBDIRS=common functions math saxon sets strings dynamic date $(CRYPTO_TESTDIR) test tests: @(cur=`pwd` ; for dir in $(SUBDIRS) ; do cd $$dir ; $(MAKE) CHECKER='$(CHECKER)' tests ; cd $$cur ; done) diff --git a/tests/exslt/saxon/Makefile.am b/tests/exslt/saxon/Makefile.am new file mode 100644 index 00000000..8535633f --- /dev/null +++ b/tests/exslt/saxon/Makefile.am @@ -0,0 +1,48 @@ +## Process this file with automake to produce Makefile.in + +$(top_builddir)/xsltproc/xsltproc: + @(cd ../../../xsltproc ; $(MAKE) xsltproc) + +EXTRA_DIST = \ + eval.1.out eval.1.xml eval.1.xsl \ + eval.2.out eval.2.xml eval.2.xsl \ + eval.3.out eval.3.xml eval.3.xsl + +CLEANFILES = .memdump + +valgrind: + @echo '## Running the regression tests under Valgrind' + $(MAKE) CHECKER='libtool --mode=execute valgrind -q --leak-check=full' tests + +test tests: $(top_builddir)/xsltproc/xsltproc + @echo '## Running exslt saxon tests' + @(echo > .memdump) + @(for i in $(srcdir)/*.xsl ; do \ + name=`basename $$i .xsl` ; \ + if [ ! -f $(srcdir)/$$name.xml ] ; then continue ; fi ; \ + log=`$(CHECKER) $(top_builddir)/xsltproc/xsltproc \ + $(srcdir)/$$name.xsl $(srcdir)/$$name.xml > $$name.res 2>$$name.bad;\ + if [ ! -f $(srcdir)/$$name.out ] ; then \ + cp $$name.res $(srcdir)/$$name.out ; \ + if [ -s $$name.bad ] ; then \ + mv $$name.bad $(srcdir)/$$name.err ; \ + fi ; \ + else \ + if [ ! -s $$name.res ] ; then \ + echo "Fatal error, no $$name.res\n" ; \ + else \ + diff $(srcdir)/$$name.out $$name.res ; \ + if [ -s $(srcdir)/$$name.err ] ; then \ + diff $(srcdir)/$$name.err $$name.bad; \ + else \ + diff /dev/null $$name.bad; \ + fi ; \ + fi ; \ + fi; \ + grep "MORY ALLO" .memdump | grep -v "MEMORY ALLOCATED : 0" || true`;\ + if [ -n "$$log" ] ; then \ + echo $$name result ; \ + echo "$$log" ; \ + fi ; \ + rm -f $$name.res $$name.bad ; \ + done) diff --git a/tests/exslt/saxon/eval.1.out b/tests/exslt/saxon/eval.1.out new file mode 100644 index 00000000..7d0c03a9 --- /dev/null +++ b/tests/exslt/saxon/eval.1.out @@ -0,0 +1,4 @@ + + + 2 + diff --git a/tests/exslt/saxon/eval.1.xml b/tests/exslt/saxon/eval.1.xml new file mode 100644 index 00000000..651887cb --- /dev/null +++ b/tests/exslt/saxon/eval.1.xml @@ -0,0 +1,3 @@ + + 1+1 + diff --git a/tests/exslt/saxon/eval.1.xsl b/tests/exslt/saxon/eval.1.xsl new file mode 100644 index 00000000..ee97a715 --- /dev/null +++ b/tests/exslt/saxon/eval.1.xsl @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + diff --git a/tests/exslt/saxon/eval.2.err b/tests/exslt/saxon/eval.2.err new file mode 100644 index 00000000..df8adfbc --- /dev/null +++ b/tests/exslt/saxon/eval.2.err @@ -0,0 +1,7 @@ +XPath error : Invalid expression +### +^ +XPath error : Invalid expression +xmlXPathCompiledEval: evaluation failed +runtime error: file ./eval.2.xsl line 11 element value-of +XPath evaluation returned no result. diff --git a/tests/exslt/saxon/eval.2.out b/tests/exslt/saxon/eval.2.out new file mode 100644 index 00000000..3048a905 --- /dev/null +++ b/tests/exslt/saxon/eval.2.out @@ -0,0 +1,2 @@ + + diff --git a/tests/exslt/saxon/eval.2.xml b/tests/exslt/saxon/eval.2.xml new file mode 100644 index 00000000..69d62f2c --- /dev/null +++ b/tests/exslt/saxon/eval.2.xml @@ -0,0 +1 @@ + diff --git a/tests/exslt/saxon/eval.2.xsl b/tests/exslt/saxon/eval.2.xsl new file mode 100644 index 00000000..a193fdd9 --- /dev/null +++ b/tests/exslt/saxon/eval.2.xsl @@ -0,0 +1,16 @@ + + + + + + + + + + + + diff --git a/tests/exslt/saxon/eval.3.err b/tests/exslt/saxon/eval.3.err new file mode 100644 index 00000000..5a87793b --- /dev/null +++ b/tests/exslt/saxon/eval.3.err @@ -0,0 +1,6 @@ +XPath error : Undefined namespace prefix +xmlXPathCompiledEval: evaluation failed +XPath error : Invalid expression +xmlXPathCompiledEval: evaluation failed +runtime error: file ./eval.3.xsl line 11 element value-of +XPath evaluation returned no result. diff --git a/tests/exslt/saxon/eval.3.out b/tests/exslt/saxon/eval.3.out new file mode 100644 index 00000000..3048a905 --- /dev/null +++ b/tests/exslt/saxon/eval.3.out @@ -0,0 +1,2 @@ + + diff --git a/tests/exslt/saxon/eval.3.xml b/tests/exslt/saxon/eval.3.xml new file mode 100644 index 00000000..69d62f2c --- /dev/null +++ b/tests/exslt/saxon/eval.3.xml @@ -0,0 +1 @@ + diff --git a/tests/exslt/saxon/eval.3.xsl b/tests/exslt/saxon/eval.3.xsl new file mode 100644 index 00000000..5f75f3c0 --- /dev/null +++ b/tests/exslt/saxon/eval.3.xsl @@ -0,0 +1,16 @@ + + + + + + + + + + + + diff --git a/win32/runtests.py b/win32/runtests.py index 6986650a..cdd7f1ec 100644 --- a/win32/runtests.py +++ b/win32/runtests.py @@ -68,6 +68,9 @@ runtests("tests/exslt/functions") print("## Running exslt math tests") runtests("tests/exslt/math") +print("## Running exslt saxon tests") +runtests("tests/exslt/saxon") + print("## Running exslt sets tests") runtests("tests/exslt/sets")