search

NodeBB-Community / nodebb-plugin-composer-redactor

Redactor Composer for NodeBB
GNU General Public License v3.0
38 stars 28 forks source link

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

Open psychobunny opened 8 years ago

psychobunny commented 8 years ago

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: https://github.com/NodeBB/NodeBB/issues/4092

yariplus commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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.