From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from krisdoz.my.domain (schwarze@localhost [127.0.0.1])
	by krisdoz.my.domain (8.14.5/8.14.5) with ESMTP id s6NF08B6016579
	for <source@mdocml.bsd.lv>; Wed, 23 Jul 2014 11:00:09 -0400 (EDT)
Received: (from schwarze@localhost)
	by krisdoz.my.domain (8.14.5/8.14.3/Submit) id s6NF08ZS028899;
	Wed, 23 Jul 2014 11:00:08 -0400 (EDT)
Date: Wed, 23 Jul 2014 11:00:08 -0400 (EDT)
Message-Id: <201407231500.s6NF08ZS028899@krisdoz.my.domain>
X-Mailinglist: mdocml-source
Reply-To: source@mdocml.bsd.lv
MIME-Version: 1.0
From: schwarze@mdocml.bsd.lv
To: source@mdocml.bsd.lv
Subject: mdocml: Security fix: After decoding numeric (\N) and one-character (\<, 
X-Mailer: activitymail 1.26, http://search.cpan.org/dist/activitymail/
Content-Type: text/plain; charset=utf-8

Log Message:
-----------
Security fix:
After decoding numeric (\N) and one-character (\<, \> etc.)
character escape sequences, do not forget to HTML-encode the
resulting ASCII character.  Malicious manuals were able to smuggle 
XSS content by roff-escaping the HTML-special characters they need.
That's a classic bug type in many web applications, actually...  :-(

Found myself while auditing the HTML formatter for safe output handling.

Modified Files:
--------------
    mdocml:
        chars.c
        html.c

Revision Data
-------------
Index: chars.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/chars.c,v
retrieving revision 1.57
retrieving revision 1.58
diff -Lchars.c -Lchars.c -u -p -r1.57 -r1.58
--- chars.c
+++ chars.c
@@ -127,7 +127,18 @@ mchars_num2uc(const char *p, size_t sz)
 
 	if ((i = mandoc_strntoi(p, sz, 16)) < 0)
 		return('\0');
-	/* FIXME: make sure we're not in a bogus range. */
+
+	/*
+	 * Security warning:
+	 * Never extend the range of accepted characters
+	 * to overlap with the ASCII range, 0x00-0x7F
+	 * without re-auditing the callers of this function.
+	 * Some callers might relay on the fact that we never
+	 * return ASCII characters for their escaping decisions.
+	 *
+	 * XXX Code is missing here to exclude bogus ranges.
+	 */
+
 	return(i > 0x80 && i <= 0x10FFFF ? i : '\0');
 }
 
Index: html.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/html.c,v
retrieving revision 1.158
retrieving revision 1.159
diff -Lhtml.c -Lhtml.c -u -p -r1.158 -r1.159
--- html.c
+++ html.c
@@ -110,6 +110,7 @@ static	const char	*const roffscales[SCAL
 
 static	void	 bufncat(struct html *, const char *, size_t);
 static	void	 print_ctag(struct html *, enum htmltag);
+static	int	 print_escape(char);
 static	int	 print_encode(struct html *, const char *, int);
 static	void	 print_metaf(struct html *, enum mandoc_esc);
 static	void	 print_attr(struct html *, const char *, const char *);
@@ -324,6 +325,37 @@ html_strlen(const char *cp)
 }
 
 static int
+print_escape(char c)
+{
+
+	switch (c) {
+	case '<':
+		printf("&lt;");
+		break;
+	case '>':
+		printf("&gt;");
+		break;
+	case '&':
+		printf("&amp;");
+		break;
+	case '"':
+		printf("&quot;");
+		break;
+	case ASCII_NBRSP:
+		putchar('-');
+		break;
+	case ASCII_HYPH:
+		putchar('-');
+		/* FALLTHROUGH */
+	case ASCII_BREAK:
+		break;
+	default:
+		return(0);
+	}
+	return(1);
+}
+
+static int
 print_encode(struct html *h, const char *p, int norecurse)
 {
 	size_t		 sz;
@@ -350,30 +382,8 @@ print_encode(struct html *h, const char 
 		if ('\0' == *p)
 			break;
 
-		switch (*p++) {
-		case '<':
-			printf("&lt;");
-			continue;
-		case '>':
-			printf("&gt;");
-			continue;
-		case '&':
-			printf("&amp;");
-			continue;
-		case '"':
-			printf("&quot;");
-			continue;
-		case ASCII_NBRSP:
-			putchar('-');
+		if (print_escape(*p++))
 			continue;
-		case ASCII_HYPH:
-			putchar('-');
-			/* FALLTHROUGH */
-		case ASCII_BREAK:
-			continue;
-		default:
-			break;
-		}
 
 		esc = mandoc_escape(&p, &seq, &len);
 		if (ESCAPE_ERROR == esc)
@@ -408,21 +418,22 @@ print_encode(struct html *h, const char 
 
 		switch (esc) {
 		case ESCAPE_UNICODE:
-			/* Skip passed "u" header. */
+			/* Skip past "u" header. */
 			c = mchars_num2uc(seq + 1, len - 1);
 			if ('\0' != c)
 				printf("&#x%x;", c);
 			break;
 		case ESCAPE_NUMBERED:
 			c = mchars_num2char(seq, len);
-			if ('\0' != c)
+			if ( ! ('\0' == c || print_escape(c)))
 				putchar(c);
 			break;
 		case ESCAPE_SPECIAL:
 			c = mchars_spec2cp(h->symtab, seq, len);
 			if (c > 0)
 				printf("&#%d;", c);
-			else if (-1 == c && 1 == len)
+			else if (-1 == c && 1 == len &&
+			    !print_escape(*seq))
 				putchar((int)*seq);
 			break;
 		case ESCAPE_NOSPACE:
--
 To unsubscribe send an email to source+unsubscribe@mdocml.bsd.lv