From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailout.scc.kit.edu (mailout.scc.kit.edu [129.13.185.202]) by krisdoz.my.domain (8.14.5/8.14.5) with ESMTP id r4U4DdYA020113 for ; Thu, 30 May 2013 00:13:39 -0400 (EDT) Received: from hekate.usta.de (asta-nat.asta.uni-karlsruhe.de [172.22.63.82]) by scc-mailout-02.scc.kit.edu with esmtp (Exim 4.72 #1) id 1UhuF8-0000oj-Gv; Thu, 30 May 2013 06:13:38 +0200 Received: from donnerwolke.usta.de ([172.24.96.3]) by hekate.usta.de with esmtp (Exim 4.77) (envelope-from ) id 1UhuF8-0001pU-Gy; Thu, 30 May 2013 06:13:38 +0200 Received: from iris.usta.de ([172.24.96.5] helo=usta.de) by donnerwolke.usta.de with esmtp (Exim 4.72) (envelope-from ) id 1UhuF8-0001wy-8s; Thu, 30 May 2013 06:13:38 +0200 Received: from schwarze by usta.de with local (Exim 4.77) (envelope-from ) id 1UhuF8-0007K1-73; Thu, 30 May 2013 06:13:38 +0200 Date: Thu, 30 May 2013 06:13:38 +0200 From: Ingo Schwarze To: Mike Small Cc: discuss@mdocml.bsd.lv Subject: Re: mandoc -Tps aborts on \^h\n\n Message-ID: <20130530041338.GI736@iris.usta.de> References: X-Mailinglist: mdocml-discuss Reply-To: discuss@mdocml.bsd.lv MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Hi Mike, Mike Small wrote on Wed, May 29, 2013 at 09:55:59PM -0400: > Not sure if this is covered under "clean up escape sequence handling" > or another TODO I missed, No, it isn't, you have found a so far unknown issue. > but I see an abort in mandoc 1.12.1 from > OpenBSD current from around May 3rd if I do the following: > > $ mandoc -Tps `man -w roff` > ... > 509.608 441.221 moveto > (ASCII) show > 490.358 425.832 moveto > (characters) show > 490.358 410.443 moveto > (with) show > assertion "8 != c" failed: > file "/usr/src/usr.bin/mandoc/term_ps.c", > line 997, function "ps_letter" > Abort trap (core dumped) Indeed, easily reproducible. > $ man -w roff > /usr/local/man/cat7/roff.0 > /usr/share/man/man7/roff.7 > > So you also get it like this... > > $ mandoc -Tps /usr/local/man/cat7/roff.0 That doesn't make a lot of sense - you are handing an already formatted document to the parsers again. Anyway, mandoc(1) is not supposed to crash even when given garbage input. [...] > A minimal test file causing the same assertion... > > $ od -cb small_test_file.1 > 0000000 \ \b x \n \n > 040 134 010 170 012 012 > 0000006 Indeed, that works as well. It even works when you put a proper mdoc(7) or man(7) header in front of the exploit code. I have just committed a proper fix to both OpenBSD and bsd.lv, see below for the patch. Thanks for the perfect report! Ingo ----- 8< ----- schnipp ----- >8 ----- 8< ----- schnapp ----- >8 ----- Log Message: ----------- Reject non-printable characters found in the input stream even when preceded by a backslash; otherwise, the escape sequence would later be identified as invalid and the non-printable character would be passed through to the output backends, sometimes triggering assertions. Reported by Mike Small on the mdocml discuss list. Modified Files: -------------- mdocml: read.c Revision Data ------------- Index: read.c =================================================================== RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/read.c,v retrieving revision 1.34 retrieving revision 1.35 diff -Lread.c -Lread.c -u -p -r1.34 -r1.35 --- read.c +++ read.c @@ -1,7 +1,7 @@ /* $Id$ */ /* * Copyright (c) 2008, 2009, 2010, 2011 Kristaps Dzonsons - * Copyright (c) 2010, 2011, 2012 Ingo Schwarze + * Copyright (c) 2010, 2011, 2012, 2013 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 @@ -328,6 +328,15 @@ mparse_buf_r(struct mparse *curp, struct break; } + /* + * Make sure we have space for at least + * one backslash and one other character + * and the trailing NUL byte. + */ + + if (pos + 2 >= (int)ln.sz) + resize_buf(&ln, 256); + /* * Warn about bogus characters. If you're using * non-ASCII encoding, you're screwing your @@ -344,8 +353,6 @@ mparse_buf_r(struct mparse *curp, struct mandoc_msg(MANDOCERR_BADCHAR, curp, curp->line, pos, NULL); i++; - if (pos >= (int)ln.sz) - resize_buf(&ln, 256); ln.buf[pos++] = '?'; continue; } @@ -353,8 +360,6 @@ mparse_buf_r(struct mparse *curp, struct /* Trailing backslash = a plain char. */ if ('\\' != blk.buf[i] || i + 1 == (int)blk.sz) { - if (pos >= (int)ln.sz) - resize_buf(&ln, 256); ln.buf[pos++] = blk.buf[i++]; continue; } @@ -396,10 +401,20 @@ mparse_buf_r(struct mparse *curp, struct break; } - /* Some other escape sequence, copy & cont. */ + /* Catch escaped bogus characters. */ - if (pos + 1 >= (int)ln.sz) - resize_buf(&ln, 256); + c = (unsigned char) blk.buf[i+1]; + + if ( ! (isascii(c) && + (isgraph(c) || isblank(c)))) { + mandoc_msg(MANDOCERR_BADCHAR, curp, + curp->line, pos, NULL); + i += 2; + ln.buf[pos++] = '?'; + continue; + } + + /* Some other escape sequence, copy & cont. */ ln.buf[pos++] = blk.buf[i++]; ln.buf[pos++] = blk.buf[i++]; -- To unsubscribe send an email to source+unsubscribe@mdocml.bsd.lv -- To unsubscribe send an email to discuss+unsubscribe@mdocml.bsd.lv