[docbook2mdoc] Segfault encountering href attr in xml preamble(s)

[docbook2mdoc] Segfault encountering href attr in xml preamble(s)

[Stephen Gregoratto]

I tried testing the new 1.0.0 release on some of my files and came 
across this bug. Some refentry files encode their chosen xsl stylesheet 
by putting it in the file itself. I believe this is to help converters 
like xmlto(1). The format is:

<?xml-stylesheet type="text/xsl"
   href="http://foo.tld/bar.xsl"?>

When docbook2mdoc encounters the href attribute, it eventually calls 
xml_attrkey() in parse.c, failing in the TAILQ_INSERT_TAIL macro. Having 
stepped through the code in GDB, I believe this is because the queue 
hasn't been allocated yet.

I also realised that this happens on any xml declaration. The smallest - 
and of course invalid - file that triggers this behaviour is:

  <?xml href="foo"?>

or even smaller:

  <? href="foo"?>
-- 
Stephen Gregoratto
PGP: 3FC6 3D0E 2801 C348 1C44 2D34 A80C 0F8E 8BAB EC8B
--
 To unsubscribe send an email to tech+unsubscribe@mandoc.bsd.lv

Re: [docbook2mdoc] Segfault encountering href attr in xml preamble(s)

[Ingo Schwarze]

Hi Stephen,

Stephen Gregoratto wrote on Fri, Apr 19, 2019 at 02:28:33PM +1000:

> I tried testing the new 1.0.0 release on some of my files and came 
> across this bug. Some refentry files encode their chosen xsl stylesheet 
> by putting it in the file itself. I believe this is to help converters 
> like xmlto(1). The format is:
> 
> <?xml-stylesheet type="text/xsl"
>    href="http://foo.tld/bar.xsl"?>
> 
> When docbook2mdoc encounters the href attribute, it eventually calls 
> xml_attrkey() in parse.c, failing in the TAILQ_INSERT_TAIL macro.

Good analysis, thanks.

Calling xml_attrkey() is still fine.
The string "<?xml-stylesheet" is supposed to be interpreted as the
beginning of an element; docbook2mdoc doesn't really distinguish
between XML elements and XML processing instructions.  And the
string "href=" is supposed to be interpreted as the beginning of
an attribute.

But inside xml_attrkey(), no processing should be done because
an unknown element does not generate a node in the node tree,
so there is no node that the attribute could be inserted into.

I think i broke this when making the NODE_* enum constants in node.h
more fine-grained, specifically when distinguishing NODE_UNKNOWN
and NODE_IGNORE.

Fixed with the commit below.

Yours,
  Ingo


Log Message:
-----------
do not crash by trying to insert an attribute into a non-existent node;
segfault reported by Stephen Gregoratto <dev at sgregoratto dot me>

Modified Files:
--------------
    docbook2mdoc:
        parse.c

Revision Data
-------------
Index: parse.c
===================================================================
RCS file: /home/cvs/mdocml/docbook2mdoc/parse.c,v
retrieving revision 1.46
retrieving revision 1.47
diff -Lparse.c -Lparse.c -u -p -r1.46 -r1.47
--- parse.c
+++ parse.c
@@ -553,7 +553,7 @@ xml_attrkey(struct parse *p, const char 
 	const char	*value;
 	enum attrkey	 key;
 
-	if (p->del > 0 || p->ncur == NODE_IGNORE || *name == '\0')
+	if (p->del > 0 || p->ncur >= NODE_UNKNOWN || *name == '\0')
 		return;
 
 	if ((p->ncur == NODE_DOCTYPE || p->ncur == NODE_ENTITY) &&
@@ -590,7 +590,7 @@ xml_attrval(struct parse *p, const char 
 {
 	struct pattr	*a;
 
-	if (p->del > 0 || p->ncur == NODE_IGNORE ||
+	if (p->del > 0 || p->ncur >= NODE_UNKNOWN ||
 	    (p->flags & PFLAG_ATTR) == 0)
 		return;
 	if ((a = TAILQ_LAST(&p->cur->attrq, pattrq)) == NULL)
--
 To unsubscribe send an email to tech+unsubscribe@mandoc.bsd.lv