source@mandoc.bsd.lv
 help / color / mirror / Atom feed
From: schwarze@mandoc.bsd.lv
To: source@mandoc.bsd.lv
Subject: mandoc: Fix a buffer overrun in the roff(7) escape sequence parser that
Date: Wed, 1 Jun 2022 18:20:56 -0500 (EST)	[thread overview]
Message-ID: <336598182eb582a7@mandoc.bsd.lv> (raw)

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


                 reply	other threads:[~2022-06-01 23:20 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=336598182eb582a7@mandoc.bsd.lv \
    --to=schwarze@mandoc.bsd.lv \
    --cc=source@mandoc.bsd.lv \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).