* mdocml: Security fix: After decoding numeric (\N) and one-character (\<,
@ 2014-07-23 15:00 schwarze
0 siblings, 0 replies; only message in thread
From: schwarze @ 2014-07-23 15:00 UTC (permalink / raw)
To: source
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("<");
+ break;
+ case '>':
+ printf(">");
+ break;
+ case '&':
+ printf("&");
+ break;
+ case '"':
+ printf(""");
+ 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("<");
- continue;
- case '>':
- printf(">");
- continue;
- case '&':
- printf("&");
- continue;
- case '"':
- printf(""");
- 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
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2014-07-23 15:00 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-23 15:00 mdocml: Security fix: After decoding numeric (\N) and one-character (\<, schwarze
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).