From f4269c9f5eb96ceb250e223c7e073a0650c3b439 Mon Sep 17 00:00:00 2001 From: schwarze Date: Sat, 20 Aug 2016 17:58:09 +0000 Subject: [PATCH] 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. --- regress/usr.bin/mandoc/mdoc/break/Makefile | 8 ++-- regress/usr.bin/mandoc/mdoc/break/notopen.in | 12 ++++++ .../mandoc/mdoc/break/notopen.out_ascii | 9 +++++ .../mandoc/mdoc/break/notopen.out_lint | 1 + usr.bin/mandoc/mdoc_macro.c | 37 ++++++++++++------- 5 files changed, 49 insertions(+), 18 deletions(-) create mode 100644 regress/usr.bin/mandoc/mdoc/break/notopen.in create mode 100644 regress/usr.bin/mandoc/mdoc/break/notopen.out_ascii create mode 100644 regress/usr.bin/mandoc/mdoc/break/notopen.out_lint diff --git a/regress/usr.bin/mandoc/mdoc/break/Makefile b/regress/usr.bin/mandoc/mdoc/break/Makefile index e91e20f80fd..b4c3b219163 100644 --- 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 diff --git a/regress/usr.bin/mandoc/mdoc/break/notopen.in b/regress/usr.bin/mandoc/mdoc/break/notopen.in new file mode 100644 index 00000000000..20a9bf1291f --- /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 index 00000000000..db71cc41743 --- /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) + +NNAAMMEE + bbrreeaakk--nnoottooppeenn - mismatching end macro inside two open blocks + +DDEESSCCRRIIPPTTIIOONN + 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 index 00000000000..e70b2f6fe85 --- /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 331aeb52903..bb0a07e83fe 100644 --- 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 - * Copyright (c) 2010, 2012-2015 Ingo Schwarze + * Copyright (c) 2010, 2012-2016 Ingo Schwarze * * 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: -- 2.20.1