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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XSS in chat because markdown is required to be disabled #37

Open
psychobunny opened this issue Feb 24, 2016 · 3 comments
Open

XSS in chat because markdown is required to be disabled #37

psychobunny opened this issue Feb 24, 2016 · 3 comments

Comments

@psychobunny
Copy link
Contributor

The problem is that markdown sanitzes both posts and chat content, and so because redactor comes with it's own parser, markdown has to be disabled. I wonder if there's a way to make them both play nicely together...

Original issue:
NodeBB/NodeBB#4092

@yariplus
Copy link
Member

I don't see how this is an issue.

The same problem happens in default composer if markdown allows html.

It's not the composer's job to police XSS where the composer has no involvement.

Either add basic XSS prevention in core, or require the sanitizehtml plugin to be installed.

@psychobunny
Copy link
Contributor Author

Well,

  1. markdown has a warning in ACP against allowing HTML
  2. anybody using this plugin has the right to know that they will be susceptible to XSS via chats

That said, you're right in the sense that it may not be this plugin's issue to fix. So maybe having a warning in the README and then move this issue back to core is the right thing to do?

@yariplus
Copy link
Member

yariplus commented Sep 1, 2016

Yes, that definitely sounds good.

I would even consider adding a link to sanitizehtml in the markdown html warning. Although I still think adding basic XSS protection in core is the best solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants