Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security feature request: a setting to block inline javascript #465

Closed
winterstein opened this issue Sep 18, 2020 · 4 comments 路 Fixed by #563
Closed

Security feature request: a setting to block inline javascript #465

winterstein opened this issue Sep 18, 2020 · 4 comments 路 Fixed by #563
Labels
馃憖 no/external This makes more sense somewhere else

Comments

@winterstein
Copy link

Firstly, thank you for this useful package 馃憤

We want to allow html (so escapeHtml={false}) -- but not allow onClick, onMouseOver, onX etc handlers -- as these would allow an attacker to inject javascript traps.

I don't think there is a way to do this. So I'm requesting a feature to block inline javascript.

Meanwhile, we can achieve this by pre-filtering the input with a few regexs.

@codefull
Copy link

Firstly, thank you for this useful package 馃憤

We want to allow html (so escapeHtml={false}) -- but not allow onClick, onMouseOver, onX etc handlers -- as these would allow an attacker to inject javascript traps.

I don't think there is a way to do this. So I'm requesting a feature to block inline javascript.

Meanwhile, we can achieve this by pre-filtering the input with a few regexs.

Hi, could you please share your approach to filter those elements.

@winterstein
Copy link
Author

Sure - we're sniffing for onX in a tag. If we find it, we re-enable escapeHtml for safety:

// security: no onClick etc traps	
if ( ! escapeHtml) {
	let bad = source.match(/<[^>]+\bon[a-zA-Z]+=/g, ''); // match e.g. onClick within a tag
	if (bad) {
		console.warn("Dangerous content in markdown!", bad, source);
	}
	escapeHtml = true;
}

@ChristianMurphy
Copy link
Member

This would be supported more directly through #428, which supports https://github.com/rehypejs/rehype-sanitize

@ChristianMurphy ChristianMurphy added the 馃憖 no/external This makes more sense somewhere else label Oct 8, 2020
wooorm added a commit that referenced this issue Apr 12, 2021
* Replace `renderers` w/ `components`
* Replace `allowNode` w/ `allowElement`, which is now given a hast element (as
  the first parameter)
* Replace `allowedTypes` w/ `allowedElements`
* Replace `disallowedTypes` w/ `disallowedElements`
* Change signature of `linkTarget` and `transformLinkUri`, which are now given
  hast children (as the second parameter)
* Change signature of `transformImageUri`, which is now given the `alt` string
  as the second parameter (instead of the fourth)
* Replace `plugins` w/ `remarkPlugins` (backwards compatible change)
* Add `rehypePlugins`
* Change `includeNodeIndex` to `includeElementIndex`: it still sets an `index`,
  but that value now represents the number of preceding elements, it also sets a
  `siblingCount` (instead of `parentChildCount`) with the number of sibling
  elements in the parent
* The `columnAlignment` prop is no longer given to table elements: it鈥檚
  available as `style` on `th` and `td` elements instead
* The `spread` prop is no longer given to list elements: it鈥檚 already handled

Remove buggy HTML parsers from core

* If you want HTML, add [`rehype-raw`](https://github.com/rehypejs/rehype-raw)
  to `rehypePlugins` and it鈥檒l work without bugs!
* Remove `allowDangerousHtml` (previously called `escapeHtml`) option: pass
  `rehype-raw` in `rehypePlugins` to allow HTML instead
* Remove `with-html.js`, `plugins/html-parser.js` entries from library
* Remove na茂ve HTML parser too: either use `rehype-raw` to properly support
  HTML, or don鈥檛 allow it at all

Closes GH-549.
Closes GH-563.

The following issues are solved as rehype is now available:

Closes GH-522.
Closes GH-465.
Closes GH-427.
Closes GH-384.
Closes GH-356.

The following issues are solved as a proper HTML parser (`rehype-raw`) is now
available:

Closes GH-562.
Closes GH-460.
Closes GH-454.
Closes GH-452.
Closes GH-433.
Closes GH-386.
Closes GH-385.
Closes GH-345.
Closes GH-320.
Closes GH-302.
Closes GH-267.
Closes GH-259.

The following issues are solved as docs are improved:

Closes GH-251.
@wooorm
Copy link
Member

wooorm commented Apr 12, 2021

This should be solved by landing GH-563 today, which will soon be released in v6.0.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
馃憖 no/external This makes more sense somewhere else
Development

Successfully merging a pull request may close this issue.

4 participants