From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-0.0 required=5.0 tests=T_SCC_BODY_TEXT_LINE, UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 23399 invoked from network); 1 Jun 2022 23:20:59 -0000 Received: from bsd.lv (HELO mandoc.bsd.lv) (66.111.2.12) by inbox.vuxu.org with ESMTPUTF8; 1 Jun 2022 23:20:59 -0000 Received: from fantadrom.bsd.lv (localhost [127.0.0.1]) by mandoc.bsd.lv (OpenSMTPD) with ESMTP id 91cd7af4 for ; Wed, 1 Jun 2022 18:20:56 -0500 (EST) Received: from localhost (mandoc.bsd.lv [local]) by mandoc.bsd.lv (OpenSMTPD) with ESMTPA id ff065ffc for ; Wed, 1 Jun 2022 18:20:56 -0500 (EST) Date: Wed, 1 Jun 2022 18:20:56 -0500 (EST) X-Mailinglist: mandoc-source Reply-To: source@mandoc.bsd.lv MIME-Version: 1.0 From: schwarze@mandoc.bsd.lv To: source@mandoc.bsd.lv Subject: mandoc: Fix a buffer overrun in the roff(7) escape sequence parser that X-Mailer: activitymail 1.26, http://search.cpan.org/dist/activitymail/ Content-Type: text/plain; charset=utf-8 Message-ID: <336598182eb582a7@mandoc.bsd.lv> Log Message: ----------- Fix a buffer overrun in the roff(7) escape sequence parser that could be triggered by macro arguments ending in double backslashes, for example if people wrote .Sq "\\" instead of the correct .Sq "\e". The bug was hard to find because it caused a segfault only very rarely, according to my measurements with a probability of less than one permille. I'm sorry that the first one to hit the bug was an arm64 release build run by deraadt@. Thanks to bluhm@ for providing access to an arm64 machine for debugging purposes. In the end, the bug turned out to be architecture-independent. The reason for the bug was that i assumed an invariant that does not exist. The function roff_parse_comment() is very careful to make sure that the input buffer does not end in an escape character before passing it on, so i assumed this is still true when reaching roff_expand() immediately afterwards. But roff_expand() can also be reached from roff_getarg(), in which case there *can* be a lone escape character at the end of the buffer in case copy mode processing found and converted a double backslash. Fix this by handling a trailing escape character correctly in the function roff_escape(). The lesson here probably is to refrain from assuming an invariant unless verifying that the invariant actually holds is reasonably simple. In some cases, in particular for invariants that are important but not simple, it might also make sense to assert(3) rather than just assume the invariant. An assertion failure is so much better than a buffer overrun... Modified Files: -------------- mandoc: roff_escape.c Revision Data ------------- Index: roff_escape.c =================================================================== RCS file: /home/cvs/mandoc/mandoc/roff_escape.c,v retrieving revision 1.5 retrieving revision 1.6 diff -Lroff_escape.c -Lroff_escape.c -u -p -r1.5 -r1.6 --- roff_escape.c +++ roff_escape.c @@ -124,6 +124,9 @@ roff_escape(const char *buf, const int l rval = ESCAPE_IGNORE; goto out; + case '\0': + iendarg = --iend; + /* FALLTHROUGH */ case '\\': default: iarg--; -- To unsubscribe send an email to source+unsubscribe@mandoc.bsd.lv