When a mismatching end macro occurs while at least two nested blocks

are open, all except the innermost open block got a bogus MDOC_ENDED
marker, in some situations triggering segfaults down the road
which tb@ found with afl(1).
Fix the logic error by figuring out up front whether an end macro
has a matching body, and if it hasn't, don't mark any blocks as broken.

diff --git a/regress/usr.bin/mandoc/mdoc/break/Makefile b/regress/usr.bin/mandoc/mdoc/break/Makefile
index e91e20f..b4c3b21 100644 (file)
--- a/regress/usr.bin/mandoc/mdoc/break/Makefile
+++ b/regress/usr.bin/mandoc/mdoc/break/Makefile
@@ -1,7 +1,7 @@
-# $OpenBSD: Makefile,v 1.3 2015/04/05 14:43:10 schwarze Exp $
+# $OpenBSD: Makefile,v 1.4 2016/08/20 17:58:09 schwarze Exp $
 
-REGRESS_TARGETS        = brokenbreaker twice tail two
-LINT_TARGETS   = brokenbreaker twice tail two
+REGRESS_TARGETS        = brokenbreaker twice tail two notopen
+LINT_TARGETS   = brokenbreaker twice tail two notopen
 
 # It's hard to keep stuff together in next-line scope.
 
@@ -10,6 +10,6 @@ SKIP_TMAN     = tail
 # groff-1.22.3 defect:
 # - non-matching enclosure end macro prints a closing delimiter
 
-SKIP_GROFF     = brokenbreaker
+SKIP_GROFF     = brokenbreaker notopen
 
 .include <bsd.regress.mk>

diff --git a/regress/usr.bin/mandoc/mdoc/break/notopen.in b/regress/usr.bin/mandoc/mdoc/break/notopen.in
new file mode 100644 (file)
index 0000000..20a9bf1
--- /dev/null
+++ b/regress/usr.bin/mandoc/mdoc/break/notopen.in
@@ -0,0 +1,12 @@
+.Dd August 20, 2016
+.Dt BREAK-NOTOPEN 1
+.Os OpenBSD
+.Sh NAME
+.Nm break-notopen
+.Nd mismatching end macro inside two open blocks
+.Sh DESCRIPTION
+.Ao ao
+.Bo bo pc
+.Pc bc
+.Bc ac
+.Ac tail

diff --git a/regress/usr.bin/mandoc/mdoc/break/notopen.out_ascii b/regress/usr.bin/mandoc/mdoc/break/notopen.out_ascii
new file mode 100644 (file)
index 0000000..db71cc4
--- /dev/null
+++ b/regress/usr.bin/mandoc/mdoc/break/notopen.out_ascii
@@ -0,0 +1,9 @@
+BREAK-NOTOPEN(1)            General Commands Manual           BREAK-NOTOPEN(1)
+
+N\bNA\bAM\bME\bE
+     b\bbr\bre\bea\bak\bk-\b-n\bno\bot\bto\bop\bpe\ben\bn - mismatching end macro inside two open blocks
+
+D\bDE\bES\bSC\bCR\bRI\bIP\bPT\bTI\bIO\bON\bN
+     <ao [bo pc bc] ac> tail
+
+OpenBSD                         August 20, 2016                        OpenBSD

diff --git a/regress/usr.bin/mandoc/mdoc/break/notopen.out_lint b/regress/usr.bin/mandoc/mdoc/break/notopen.out_lint
new file mode 100644 (file)
index 0000000..e70b2f6
--- /dev/null
+++ b/regress/usr.bin/mandoc/mdoc/break/notopen.out_lint
@@ -0,0 +1 @@
+mandoc: notopen.in:10:2: ERROR: skipping end of block that is not open: Pc

diff --git a/usr.bin/mandoc/mdoc_macro.c b/usr.bin/mandoc/mdoc_macro.c
index 331aeb5..bb0a07e 100644 (file)
--- a/usr.bin/mandoc/mdoc_macro.c
+++ b/usr.bin/mandoc/mdoc_macro.c
@@ -1,7 +1,7 @@
-/*     $OpenBSD: mdoc_macro.c,v 1.164 2016/08/20 15:58:16 schwarze Exp $ */
+/*     $OpenBSD: mdoc_macro.c,v 1.165 2016/08/20 17:58:09 schwarze Exp $ */
 /*
  * Copyright (c) 2008-2012 Kristaps Dzonsons <kristaps@bsd.lv>
- * Copyright (c) 2010, 2012-2015 Ingo Schwarze <schwarze@openbsd.org>
+ * Copyright (c) 2010, 2012-2016 Ingo Schwarze <schwarze@openbsd.org>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -547,13 +547,24 @@ blk_exp_close(MACRO_PROT_ARGS)
                break;
        }
 
+       /* Search backwards for the beginning of our own body. */
+
+       atok = rew_alt(tok);
+       body = NULL;
+       for (n = mdoc->last; n; n = n->parent) {
+               if (n->flags & MDOC_ENDED || n->tok != atok ||
+                   n->type != ROFFT_BODY || n->end != ENDBODY_NOT)
+                       continue;
+               body = n;
+               break;
+       }
+
        /*
         * Search backwards for beginnings of blocks,
         * both of our own and of pending sub-blocks.
         */
 
-       atok = rew_alt(tok);
-       body = endbody = itblk = later = NULL;
+       endbody = itblk = later = NULL;
        for (n = mdoc->last; n; n = n->parent) {
                if (n->flags & MDOC_ENDED) {
                        if ( ! (n->flags & MDOC_VALID))
@@ -561,15 +572,15 @@ blk_exp_close(MACRO_PROT_ARGS)
                        continue;
                }
 
-               /* Remember the start of our own body. */
-
-               if (n->type == ROFFT_BODY && atok == n->tok) {
-                       if (n->end == ENDBODY_NOT)
-                               body = n;
-                       continue;
-               }
+               /*
+                * Mismatching end macros can never break anything,
+                * SYNOPSIS name blocks can never be broken,
+                * and we only care about the breaking of BLOCKs.
+                */
 
-               if (n->type != ROFFT_BLOCK || n->tok == MDOC_Nm)
+               if (body == NULL ||
+                   n->tok == MDOC_Nm ||
+                   n->type != ROFFT_BLOCK)
                        continue;
 
                if (n->tok == MDOC_It) {
@@ -637,8 +648,6 @@ blk_exp_close(MACRO_PROT_ARGS)
        if (body == NULL) {
                mandoc_msg(MANDOCERR_BLK_NOTOPEN, mdoc->parse,
                    line, ppos, mdoc_macronames[tok]);
-               if (later != NULL)
-                       later->flags &= ~MDOC_BROKEN;
                if (maxargs && endbody == NULL) {
                        /*
                         * Stray .Ec without previous .Eo: