Skip to content

Commit

Permalink
Sanitize hardening (#1504)
Browse files Browse the repository at this point in the history
Sanitize hardening

Co-authored-by: Tony Brix <tony@brix.ninja>
  • Loading branch information
UziTech committed Jul 2, 2019
2 parents 124c576 + b78e498 commit 2a1bbed
Show file tree
Hide file tree
Showing 13 changed files with 53 additions and 5 deletions.
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -39,7 +39,7 @@ Also read about:

## Usage

### Warning: 🚨 Marked does not [sanitize](https://marked.js.org/#/USING_ADVANCED.md#options) the output HTML by default 🚨
### Warning: 🚨 Marked does not [sanitize](https://marked.js.org/#/USING_ADVANCED.md#options) the output HTML. Please use a sanitize library, like [DOMPurify](https://github.com/cure53/DOMPurify) (recommended), [sanitize-html](https://github.com/apostrophecms/sanitize-html) or [insane](https://github.com/bevacqua/insane) on the output HTML! 🚨

**CLI**

Expand Down
2 changes: 1 addition & 1 deletion docs/README.md
Expand Up @@ -25,7 +25,7 @@ These documentation pages are also rendered using marked 💯

<h2 id="usage">Usage</h2>

### Warning: 🚨 Marked does not [sanitize](https://marked.js.org/#/USING_ADVANCED.md#options) the output HTML by default 🚨
### Warning: 🚨 Marked does not [sanitize](https://marked.js.org/#/USING_ADVANCED.md#options) the output HTML. Please use a sanitize library, like [DOMPurify](https://github.com/cure53/DOMPurify) (recommended), [sanitize-html](https://github.com/apostrophecms/sanitize-html) or [insane](https://github.com/bevacqua/insane) on the output HTML! 🚨

**CLI**

Expand Down
2 changes: 1 addition & 1 deletion docs/USING_ADVANCED.md
Expand Up @@ -51,7 +51,7 @@ console.log(marked(markdownString));
|mangle |`boolean` |`true` |v0.3.4 |If true, autolinked email address is escaped with HTML character references.|
|pedantic |`boolean` |`false` |v0.2.1 |If true, conform to the original `markdown.pl` as much as possible. Don't fix original markdown bugs or behavior. Turns off and overrides `gfm`.|
|renderer |`object` |`new Renderer()`|v0.3.0|An object containing functions to render tokens to HTML. See [extensibility](USING_PRO.md) for more details.|
|sanitize |`boolean` |`false` |v0.2.1 |If true, sanitize the HTML passed into `markdownString` with the `sanitizer` function.|
|sanitize |`boolean` |`false` |v0.2.1 |If true, sanitize the HTML passed into `markdownString` with the `sanitizer` function.<br>**Warning**: This feature is deprecated and it should NOT be used as it cannot be considered secure.<br>Instead use a sanitize library, like [DOMPurify](https://github.com/cure53/DOMPurify) (recommended), [sanitize-html](https://github.com/apostrophecms/sanitize-html) or [insane](https://github.com/bevacqua/insane) on the output HTML! |
|sanitizer |`function`|`null` |v0.3.4 |A function to sanitize the HTML passed into `markdownString`.|
|silent |`boolean` |`false` |v0.2.7 |If true, the parser does not throw any exception.|
|smartLists |`boolean` |`false` |v0.2.8 |If true, use smarter list behavior than those found in `markdown.pl`.|
Expand Down
12 changes: 10 additions & 2 deletions lib/marked.js
Expand Up @@ -434,7 +434,7 @@ Lexer.prototype.token = function(src, top) {
: 'html',
pre: !this.options.sanitizer
&& (cap[1] === 'pre' || cap[1] === 'script' || cap[1] === 'style'),
text: cap[0]
text: this.options.sanitize ? (this.options.sanitizer ? this.options.sanitizer(cap[0]) : escape(cap[0])) : cap[0]
});
continue;
}
Expand Down Expand Up @@ -850,7 +850,7 @@ InlineLexer.prototype.output = function(src) {
if (cap = this.rules.text.exec(src)) {
src = src.substring(cap[0].length);
if (this.inRawBlock) {
out += this.renderer.text(cap[0]);
out += this.renderer.text(this.options.sanitize ? (this.options.sanitizer ? this.options.sanitizer(cap[0]) : escape(cap[0])) : cap[0]);
} else {
out += this.renderer.text(escape(this.smartypants(cap[0])));
}
Expand Down Expand Up @@ -1539,6 +1539,12 @@ function findClosingBracket(str, b) {
return -1;
}

function checkSanitizeDeprecation(opt) {
if (opt && opt.sanitize && !opt.silent) {
console.warn('marked(): sanitize and sanitizer parameters are deprecated since version 0.7.0, should not be used and will be removed in the future. Read more here: https://marked.js.org/#/USING_ADVANCED.md#options');
}
}

/**
* Marked
*/
Expand All @@ -1560,6 +1566,7 @@ function marked(src, opt, callback) {
}

opt = merge({}, marked.defaults, opt || {});
checkSanitizeDeprecation(opt);

var highlight = opt.highlight,
tokens,
Expand Down Expand Up @@ -1624,6 +1631,7 @@ function marked(src, opt, callback) {
}
try {
if (opt) opt = merge({}, marked.defaults, opt);
checkSanitizeDeprecation(opt);
return Parser.parse(Lexer.lex(src, opt), opt);
} catch (e) {
e.message += '\nPlease report this to https://github.com/markedjs/marked.';
Expand Down
5 changes: 5 additions & 0 deletions test/specs/run-spec.js
Expand Up @@ -16,6 +16,10 @@ function runSpecs(title, dir, showCompletionTable, options) {
spec.options = Object.assign({}, options, (spec.options || {}));
const example = (spec.example ? ' example ' + spec.example : '');
const passFail = (spec.shouldFail ? 'fail' : 'pass');
if (spec.options.sanitizer) {
// eslint-disable-next-line no-eval
spec.options.sanitizer = eval(spec.options.sanitizer);
}
(spec.only ? fit : it)('should ' + passFail + example, () => {
const before = process.hrtime();
if (spec.shouldFail) {
Expand All @@ -40,3 +44,4 @@ runSpecs('CommonMark', './commonmark', true, { headerIds: false });
runSpecs('Original', './original', false, { gfm: false });
runSpecs('New', './new');
runSpecs('ReDOS', './redos');
runSpecs('Security', './security', false, { silent: true }); // silent - do not show deprecation warning
6 changes: 6 additions & 0 deletions test/specs/security/sanitizer_bypass.html
@@ -0,0 +1,6 @@
<p>AAA&lt;script&gt; &lt;img &lt;script&gt; src=x onerror=alert(1) /&gt;BBB</p>

<p>AAA&lt;sometag&gt; &lt;img &lt;sometag&gt; src=x onerror=alert(1)BBB</p>

<p>&lt;a&gt;a2&lt;a2t&gt;a2&lt;/a&gt; b &lt;c&gt;c&lt;/c&gt; d</p>
<h1 id="text"><img src="URL" alt="text"></h1>
9 changes: 9 additions & 0 deletions test/specs/security/sanitizer_bypass.md
@@ -0,0 +1,9 @@
---
sanitize: true
---
AAA<script> <img <script> src=x onerror=alert(1) />BBB

AAA<sometag> <img <sometag> src=x onerror=alert(1)BBB

<a>a2<a2t>a2</a> b <c>c</c> d
# ![text](URL)
2 changes: 2 additions & 0 deletions test/specs/security/sanitizer_bypass_remove_generic.html
@@ -0,0 +1,2 @@
<p>a2a2 b c d</p>
<h1 id="text"><img src="URL" alt="text"></h1>
6 changes: 6 additions & 0 deletions test/specs/security/sanitizer_bypass_remove_generic.md
@@ -0,0 +1,6 @@
---
sanitize: true
sanitizer: () => ''
---
<a>a2<a2t>a2</a> b <c>c</c> d
# ![text](URL)
1 change: 1 addition & 0 deletions test/specs/security/sanitizer_bypass_remove_script.html
@@ -0,0 +1 @@
<p>AAA</p>
5 changes: 5 additions & 0 deletions test/specs/security/sanitizer_bypass_remove_script.md
@@ -0,0 +1,5 @@
---
sanitize: true
sanitizer: () => ''
---
AAA<script> <img <script> src=x onerror=alert(1) />BBB
1 change: 1 addition & 0 deletions test/specs/security/sanitizer_bypass_remove_tag.html
@@ -0,0 +1 @@
<p>AAA &lt;img src=x onerror=alert(1)BBB</p>
5 changes: 5 additions & 0 deletions test/specs/security/sanitizer_bypass_remove_tag.md
@@ -0,0 +1,5 @@
---
sanitize: true
sanitizer: () => ''
---
AAA<sometag> <img <sometag> src=x onerror=alert(1)BBB

0 comments on commit 2a1bbed

Please sign in to comment.