Skip to content

Commit 325a147

Browse files
jgmjgm
committed
Make rendering safe by default.
Removes CMARK_OPT_SAFE from options. Adds CMARK_OPT_UNSAFE, with the opposite meaning. The new default behavior is to suppress raw HTML and potentially dangerous links. The CMARK_OPT_UNSAFE option has to be set explicitly to prevent this. -------------------------------------------------------- NOTE: This change will require modifications in bindings for cmark and in most libraries and programs that use cmark. -------------------------------------------------------- Closes commonmark#239, commonmark#273. Borrows heavily from @kivikakk's patch in github#123.
1 parent ca8ef74 commit 325a147

File tree

8 files changed

+32
-29
lines changed

8 files changed

+32
-29
lines changed

‎README.md‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -156,14 +156,14 @@ be found in the man pages in the `man` subdirectory.
156156
Security
157157
--------
158158

159-
By default, the library will pass through raw HTML and potentially
159+
By default, the library will scrub raw HTML and potentially
160160
dangerous links (`javascript:`, `vbscript:`, `data:`, `file:`).
161161

162-
It is recommended that users either disable this potentially unsafe
163-
feature by using the option `CMARK_OPT_SAFE` (or `--safe` with the
164-
command-line program), or run the output through an HTML sanitizer
165-
to protect against
166-
[XSS attacks](http://en.wikipedia.org/wiki/Cross-site_scripting).
162+
To allow these, use the option `CMARK_OPT_UNSAFE` (or
163+
`--unsafe`) with the command line program. If doing so, we
164+
recommend you use a HTML sanitizer specific to your needs to
165+
protect against [XSS
166+
attacks](http://en.wikipedia.org/wiki/Cross-site_scripting).
167167

168168
Contributing
169169
------------

‎api_test/main.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,8 @@ static void accessors(test_batch_runner *runner) {
177177
OK(runner, cmark_node_set_literal(string, literal + sizeof("prefix")),
178178
"set_literal suffix");
179179

180-
char *rendered_html = cmark_render_html(doc, CMARK_OPT_DEFAULT);
180+
char *rendered_html = cmark_render_html(doc,
181+
CMARK_OPT_DEFAULT | CMARK_OPT_UNSAFE);
181182
static const char expected_html[] =
182183
"<h3>Header</h3>\n"
183184
"<ol start=\"3\">\n"
@@ -859,7 +860,7 @@ static void test_safe(test_batch_runner *runner) {
859860
"a>\n[link](JAVAscript:alert('hi'))\n![image]("
860861
"file:my.js)\n";
861862
char *html = cmark_markdown_to_html(raw_html, sizeof(raw_html) - 1,
862-
CMARK_OPT_DEFAULT | CMARK_OPT_SAFE);
863+
CMARK_OPT_DEFAULT);
863864
STR_EQ(runner, html, "<!-- raw HTML omitted -->\n<p><!-- raw HTML omitted "
864865
"-->hi<!-- raw HTML omitted -->\n<a "
865866
"href=\"\">link</a>\n<img src=\"\" alt=\"image\" "

‎man/man3/cmark.3‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
.TH cmark 3 "June 02, 2017" "LOCAL" "Library Functions Manual"
1+
.TH cmark 3 "March 17, 2019" "LOCAL" "Library Functions Manual"
22
.SH
33
NAME
44
.PP
@@ -721,17 +721,17 @@ Render \f[C]softbreak\f[] elements as hard line breaks.
721721
.nf
722722
\fC
723723
.RS 0n
724-
#define CMARK_OPT_SAFE (1 << 3)
724+
#define CMARK_OPT_UNSAFE (1 << 17)
725725
.RE
726726
\f[]
727727
.fi
728728

729729
.PP
730-
Suppress raw HTML and unsafe links (\f[C]javascript:\f[],
730+
Render raw HTML and unsafe links (\f[C]javascript:\f[],
731731
\f[C]vbscript:\f[], \f[C]file:\f[], and \f[C]data:\f[], except for
732732
\f[C]image/png\f[], \f[C]image/gif\f[], \f[C]image/jpeg\f[], or
733-
\f[C]image/webp\f[] mime types). Raw HTML is replaced by a placeholder
734-
HTML comment. Unsafe links are replaced by empty strings.
733+
\f[C]image/webp\f[] mime types). By default, raw HTML is replaced by a
734+
placeholder HTML comment. Unsafe links are replaced by empty strings.
735735

736736
.PP
737737
.nf

‎src/cmark.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -552,13 +552,13 @@ char *cmark_render_latex(cmark_node *root, int options, int width);
552552
*/
553553
#define CMARK_OPT_HARDBREAKS (1 << 2)
554554

555-
/** Suppress raw HTML and unsafe links (`javascript:`, `vbscript:`,
555+
/** Render raw HTML and unsafe links (`javascript:`, `vbscript:`,
556556
* `file:`, and `data:`, except for `image/png`, `image/gif`,
557-
* `image/jpeg`, or `image/webp` mime types). Raw HTML is replaced
558-
* by a placeholder HTML comment. Unsafe links are replaced by
559-
* empty strings.
557+
* `image/jpeg`, or `image/webp` mime types). By default,
558+
* raw HTML is replaced by a placeholder HTML comment. Unsafe
559+
* links are replaced by empty strings.
560560
*/
561-
#define CMARK_OPT_SAFE (1 << 3)
561+
#define CMARK_OPT_UNSAFE (1 << 17)
562562

563563
/** Render `softbreak` elements as spaces.
564564
*/

‎src/html.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ static int S_render_node(cmark_node *node, cmark_event_type ev_type,
170170

171171
case CMARK_NODE_HTML_BLOCK:
172172
cr(html);
173-
if (options & CMARK_OPT_SAFE) {
173+
if (!(options & CMARK_OPT_UNSAFE)) {
174174
cmark_strbuf_puts(html, "<!-- raw HTML omitted -->");
175175
} else {
176176
cmark_strbuf_put(html, node->as.literal.data, node->as.literal.len);
@@ -242,7 +242,7 @@ static int S_render_node(cmark_node *node, cmark_event_type ev_type,
242242
break;
243243

244244
case CMARK_NODE_HTML_INLINE:
245-
if (options & CMARK_OPT_SAFE) {
245+
if (!(options & CMARK_OPT_UNSAFE)) {
246246
cmark_strbuf_puts(html, "<!-- raw HTML omitted -->");
247247
} else {
248248
cmark_strbuf_put(html, node->as.literal.data, node->as.literal.len);
@@ -278,8 +278,8 @@ static int S_render_node(cmark_node *node, cmark_event_type ev_type,
278278
case CMARK_NODE_LINK:
279279
if (entering) {
280280
cmark_strbuf_puts(html, "<a href=\"");
281-
if (!((options & CMARK_OPT_SAFE) &&
282-
scan_dangerous_url(&node->as.link.url, 0))) {
281+
if ((options & CMARK_OPT_UNSAFE) ||
282+
!(scan_dangerous_url(&node->as.link.url, 0))) {
283283
houdini_escape_href(html, node->as.link.url.data,
284284
node->as.link.url.len);
285285
}
@@ -296,8 +296,8 @@ static int S_render_node(cmark_node *node, cmark_event_type ev_type,
296296
case CMARK_NODE_IMAGE:
297297
if (entering) {
298298
cmark_strbuf_puts(html, "<img src=\"");
299-
if (!((options & CMARK_OPT_SAFE) &&
300-
scan_dangerous_url(&node->as.link.url, 0))) {
299+
if ((options & CMARK_OPT_UNSAFE) ||
300+
!(scan_dangerous_url(&node->as.link.url, 0))) {
301301
houdini_escape_href(html, node->as.link.url.data,
302302
node->as.link.url.len);
303303
}

‎src/main.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ void print_usage() {
3838
printf(" --sourcepos Include source position attribute\n");
3939
printf(" --hardbreaks Treat newlines as hard line breaks\n");
4040
printf(" --nobreaks Render soft line breaks as spaces\n");
41-
printf(" --safe Suppress raw HTML and dangerous URLs\n");
41+
printf(" --unsafe Render raw HTML and dangerous URLs\n");
4242
printf(" --smart Use smart punctuation\n");
4343
printf(" --validate-utf8 Replace UTF-8 invalid sequences with U+FFFD\n");
4444
printf(" --help, -h Print usage information\n");
@@ -112,8 +112,8 @@ int main(int argc, char *argv[]) {
112112
options |= CMARK_OPT_NOBREAKS;
113113
} else if (strcmp(argv[i], "--smart") == 0) {
114114
options |= CMARK_OPT_SMART;
115-
} else if (strcmp(argv[i], "--safe") == 0) {
116-
options |= CMARK_OPT_SAFE;
115+
} else if (strcmp(argv[i], "--unsafe") == 0) {
116+
options |= CMARK_OPT_UNSAFE;
117117
} else if (strcmp(argv[i], "--validate-utf8") == 0) {
118118
options |= CMARK_OPT_VALIDATE_UTF8;
119119
} else if ((strcmp(argv[i], "--help") == 0) ||

‎test/cmark-fuzz.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
1313
memcpy(&fuzz_config, data, sizeof(fuzz_config));
1414

1515
/* Mask off valid option bits */
16-
fuzz_config.options &= (CMARK_OPT_SOURCEPOS | CMARK_OPT_HARDBREAKS | CMARK_OPT_SAFE | CMARK_OPT_NOBREAKS | CMARK_OPT_NORMALIZE | CMARK_OPT_VALIDATE_UTF8 | CMARK_OPT_SMART);
16+
fuzz_config.options &= (CMARK_OPT_SOURCEPOS | CMARK_OPT_HARDBREAKS | CMARK_OPT_UNSAFE | CMARK_OPT_NOBREAKS | CMARK_OPT_NORMALIZE | CMARK_OPT_VALIDATE_UTF8 | CMARK_OPT_SMART);
1717

1818
/* Remainder of input is the markdown */
1919
const char *markdown = (const char *)(data + sizeof(fuzz_config));

‎test/cmark.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ def to_html(lib, text):
1717
markdown.argtypes = [c_char_p, c_size_t, c_int]
1818
textbytes = text.encode('utf-8')
1919
textlen = len(textbytes)
20-
result = markdown(textbytes, textlen, 0).decode('utf-8')
20+
# 1 << 17 == CMARK_OPT_UNSAFE
21+
result = markdown(textbytes, textlen, 1 << 17).decode('utf-8')
2122
return [0, result, '']
2223

2324
def to_commonmark(lib, text):
@@ -37,6 +38,7 @@ class CMark:
3738
def __init__(self, prog=None, library_dir=None):
3839
self.prog = prog
3940
if prog:
41+
prog += ' --unsafe'
4042
self.to_html = lambda x: pipe_through_prog(prog, x)
4143
self.to_commonmark = lambda x: pipe_through_prog(prog + ' -t commonmark', x)
4244
else:

0 commit comments

Comments
 (0)