From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from scc-mailout-kit-01.scc.kit.edu (scc-mailout-kit-01.scc.kit.edu [129.13.231.81]) by fantadrom.bsd.lv (OpenSMTPD) with ESMTP id c1465b4c for ; Tue, 11 Jun 2019 11:23:09 -0500 (EST) Received: from asta-nat.asta.uni-karlsruhe.de ([172.22.63.82] helo=hekate.usta.de) by scc-mailout-kit-01.scc.kit.edu with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (envelope-from ) id 1hajYA-0004Na-Rc; Tue, 11 Jun 2019 18:23:08 +0200 Received: from donnerwolke.usta.de ([172.24.96.3]) by hekate.usta.de with esmtp (Exim 4.77) (envelope-from ) id 1hajY9-0006zk-PU; Tue, 11 Jun 2019 18:23:05 +0200 Received: from athene.usta.de ([172.24.96.10]) by donnerwolke.usta.de with esmtp (Exim 4.84_2) (envelope-from ) id 1hajY9-000114-Kb; Tue, 11 Jun 2019 18:23:05 +0200 Received: from localhost (athene.usta.de [local]) by athene.usta.de (OpenSMTPD) with ESMTPA id 2d4ea59c; Tue, 11 Jun 2019 18:23:05 +0200 (CEST) Date: Tue, 11 Jun 2019 18:23:05 +0200 From: Ingo Schwarze To: Stephen Gregoratto Cc: tech@mandoc.bsd.lv Subject: Re: [mandoc] segfault due to missing tbl layout Message-ID: <20190611162305.GB78096@athene.usta.de> References: <20190604025329.4h75kp6lq7dyglcn@BlackBox> X-Mailinglist: mandoc-tech Reply-To: tech@mandoc.bsd.lv MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <20190604025329.4h75kp6lq7dyglcn@BlackBox> User-Agent: Mutt/1.8.0 (2017-02-23) Hi Stephen, Stephen Gregoratto wrote on Tue, Jun 04, 2019 at 12:53:29PM +1000: > Here's an interesting bug I found in the wild. First, some backstory. > The aerc[1] email client uses scdoc[2] to generate its manpages (same > author). Scdoc files are kinda like markdown, Bleah. Why on earth do people think using markup languages with weakly defined, highly context dependent syntax is a good idea when better languages are readily available? Anyway, that's not the point here. > but they have a funky way > of setting out tables (see the TABLES section of scdoc(5)[3]). Right. Start with a crappy input language, then convert it to low-quality output in the target language. So typical... :-( The only purpose such crap is good for is compatibility testing and finding bugs (an approach similar to fuzzing), so i added a TODO entry, see in the committed patch below. > So, the aerc-config document has a long table for command keys, and in > the middle of it the author forgot to set the alignment for a cell. I'm > attaching the formatted document, but I can trigger the same bug with > this (bug.5): >=20 > .TS > allbox; > l c > l > l l. > Foo Bar > FooBar > Foo Bar > .TE >=20 > This occurs on Arch Linux and OpenBSD 6.5, with the latest changes from > CVS. This is the output from GDB: >=20 > #0 0x000055632eb7ff52 in tbl_hrule (tp=3D0x55632edc6d00, spp=3D0x55632ed= c31a0, sp=3D0x55632edc31a0, spn=3D0x55632edc3310, flags=3D1) at tbl_term.c:= 626 > 626 col =3D tp->tbl.cols + cp->col; > (gdb) p tp->tbl.cols > $2 =3D (struct roffcol *) 0x55632edcb890 > (gdb) p cp->col > Cannot access memory at address 0x24 Thank you for the complete information and thorough analysis, that is very helpful. Yes, this was certainly a bug in mandoc, possibly introduced while improving line drawing in tables (in particular for using UTF-8 line drawing characters) a few months ago. I committed the patch below to OpenBSD and bsd.lv; i assume it fixes the bug for you, too? Yours, Ingo > I've tested this with groff and Plan 9 troff/tbl and they handle this > fine. >=20 > [1] https://git.sr.ht/~sircmpwn/aerc2 > [2] https://git.sr.ht/~sircmpwn/scdoc > [3] https://git.sr.ht/~sircmpwn/scdoc/blob/master/scdoc.5.scd > [4] https://git.sr.ht/~sircmpwn/aerc/blob/master/doc/aerc-config.5.scd Log Message: ----------- Do not access a NULL pointer if a table contains a horizontal line next to a table line having fewer columns than the table as a whole. Bug found by Stephen Gregoratto with aerc-config(5). Modified Files: -------------- mandoc: TODO tbl_term.c mandoc/regress/tbl/layout: Makefile Added Files: ----------- mandoc/regress/tbl/layout: shortlines.in shortlines.out_ascii Revision Data ------------- Index: tbl_term.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /home/cvs/mandoc/mandoc/tbl_term.c,v retrieving revision 1.70 retrieving revision 1.71 diff -Ltbl_term.c -Ltbl_term.c -u -p -r1.70 -r1.71 --- tbl_term.c +++ tbl_term.c @@ -622,8 +622,12 @@ tbl_hrule(struct termp *tp, const struct (spp =3D=3D NULL || cpn =3D=3D NULL || cpn->pos !=3D TBL_CELL_DOWN ? BRIGHT * hw : 0), 1); =20 + col =3D tp->tbl.cols; for (;;) { - col =3D tp->tbl.cols + cp->col; + if (cp =3D=3D NULL) + col++; + else + col =3D tp->tbl.cols + cp->col; =20 /* Print the horizontal line inside this column. */ =20 @@ -649,7 +653,8 @@ tbl_hrule(struct termp *tp, const struct uw =3D 1; } cpp =3D cpp->next; - } + } else if (spp !=3D NULL && opts & TBL_OPT_ALLBOX) + uw =3D 1; if (cp !=3D NULL) cp =3D cp->next; if (cpn !=3D NULL) { @@ -661,8 +666,9 @@ tbl_hrule(struct termp *tp, const struct cpn =3D cpn->next; while (dpn !=3D NULL && dpn->layout !=3D cpn) dpn =3D dpn->next; - } - if (cpp =3D=3D NULL && cp =3D=3D NULL && cpn =3D=3D NULL) + } else if (spn !=3D NULL && opts & TBL_OPT_ALLBOX) + dw =3D 1; + if (col + 1 =3D=3D tp->tbl.cols + sp->opts->cols) break; =20 /* Vertical lines do not cross spanned cells. */ Index: TODO =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /home/cvs/mandoc/mandoc/TODO,v retrieving revision 1.294 retrieving revision 1.295 diff -LTODO -LTODO -u -p -r1.294 -r1.295 --- TODO +++ TODO @@ -285,6 +285,9 @@ are mere guesses, and some may be wrong. https://github.com/schmonz/ikiwiki/compare/mandoc Amitai Schlair Mon, 19 May 2014 14:05:53 -0400 =20 +- check compatibility with + https://git.sr.ht/~sircmpwn/scdoc + - check features of the Slackware man.conf(5) format Carsten Kunze Wed, 11 Mar 2015 17:57:24 +0100 =20 Index: Makefile =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /home/cvs/mandoc/mandoc/regress/tbl/layout/Makefile,v retrieving revision 1.2 retrieving revision 1.3 diff -Lregress/tbl/layout/Makefile -Lregress/tbl/layout/Makefile -u -p -r1.= 2 -r1.3 --- regress/tbl/layout/Makefile +++ regress/tbl/layout/Makefile @@ -1,7 +1,7 @@ -# $OpenBSD: Makefile,v 1.2 2015/01/30 00:27:09 schwarze Exp $ +# $OpenBSD: Makefile,v 1.4 2019/06/11 15:40:41 schwarze Exp $ =20 REGRESS_TARGETS =3D center complex empty emptyline -REGRESS_TARGETS +=3D lines lines-nogroff numbers span +REGRESS_TARGETS +=3D lines lines-nogroff numbers shortlines span LINT_TARGETS =3D complex empty =20 # groff-1.22.3 defects: --- /dev/null +++ regress/tbl/layout/shortlines.in @@ -0,0 +1,49 @@ +.\" $OpenBSD: shortlines.in,v 1.1 2019/06/11 15:40:41 schwarze Exp $ +.TH TBL-LAYOUT-SHORTLINES 1 "June 11, 2019" +.SH NAME +tbl-layout-shortlines \- table lines of different length +.SH DESCRIPTION +normal text +.TS +allbox tab(:); +L L +L +L L. +left:right +short +left:right +.TE +.sp +.TS +allbox tab(:); +L L +L +L +L L. +left:right +first short +second short +left:right +.TE +.sp +.TS +allbox tab(:); +L L L +L L +L. +left:middle:right +short:line +very short +.TE +.sp +.TS +allbox tab(:); +L +L L +L L L. +very short +short:line +left:middle:right +.TE + + --- /dev/null +++ regress/tbl/layout/shortlines.out_ascii @@ -0,0 +1,48 @@ +TBL-LAYOUT-SHORTLINES(1) General Commands Manual TBL-LAYOUT-SHORTLINE= S(1) + + + +N=08NA=08AM=08ME=08E + tbl-layout-shortlines - table lines of different length + +D=08DE=08ES=08SC=08CR=08RI=08IP=08PT=08TI=08IO=08ON=08N + normal text + + +------+-------+ + |left | right | + +------+-------+ + |short | | + +------+-------+ + |left | right | + +------+-------+ + + +-------------+-------+ + |left | right | + +-------------+-------+ + |first short | | + +-------------+-------+ + |second short | | + +-------------+-------+ + |left | right | + +-------------+-------+ + + +-----------+--------+-------+ + |left | middle | right | + +-----------+--------+-------+ + |short | line | | + +-----------+--------+-------+ + |very short | | | + +-----------+--------+-------+ + + +-----------+--------+-------+ + |very short | | | + +-----------+--------+-------+ + |short | line | | + +-----------+--------+-------+ + |left | middle | right | + +-----------+--------+-------+ + + + + +OpenBSD June 11, 2019 TBL-LAYOUT-SHORTLINE= S(1) -- To unsubscribe send an email to tech+unsubscribe@mandoc.bsd.lv