Protect users from gallery.cnf

[oparoz]

Fixes #295

• Enabled the sanitizing option of marked
• Added DOMPurify
• Added custom rules to DOMPurify

@LukasReschke

[oparoz]

So, @LukasReschke. Is this working for you?
I tested with your example and a few others, but you probably have better tools :)

[oparoz]

Rebased

[oparoz]

@karlitschek @DeepDiver1975 Needs to be reviewed today. Feel free to delegate to someone who can.

[karlitschek]

@LukasReschke @PVince81 @schiesbn can you help to review?

[LukasReschke]

👎 - the sanitize function of marked is not secure enough please use commonmark.js as advised at #295 (comment)

Try something like:

---
# Gallery configuration file
information:
  description: This <script>alert(1)</script>is an **album description** which is only shown if there is no `description_link`
  copyright: Copyright 2003-2015 [URL](javascript&#58document;alert&#40;1&#41;)
  inherit: yes
sorting:
  type: date
  order: des
  inherit: yes
features:
  external_shares: yes
  native_svg: yes

Note that DOMPurify does just nothing in IE 8 :-)

[LukasReschke]

Or even more secure remove the feature 🙊 🙈 🙉 runs away and hides ;-)

[oparoz]

The example you gave is neutralized. The first part is converted to text and the second one is completely removed.

In the link you gave, to the list of PRs, there is only one which is relevant, but which is caught by DOMpurify:
markedjs/marked#592

I'll check commonmark to see if there are no rendering issues. There is also mardown-it, which also cares about security. They've all made good progress recently regarding speed.

Looking at DOMPurify's doc, I can confirm that I need to remove Gallery configuration support for IE8-9. Not a big deal.

[LukasReschke]

No. DOMPurify is not a proper solution. The Markdown parser on it's own should be secure as well. It's like relying on an AV to make your PC more secure. It might be nice to have but it is in no way a proper defense in depth.

I'll check commonmark to see if there are no rendering issues.

👍

There is also markdown-it, which also cares about security. They've all made good progress recently regarding speed.

googles for markdown-it

[oparoz]

OK!

[oparoz]

Done. No problem with speed with the type of content being displayed.

@LukasReschke Let me know if that now meets your requirements.

[LukasReschke]

Can we use DOMPurity 0.6.7 (or directly 0.7.0) as it fixes a bypass?

Ref https://github.com/cure53/DOMPurify/releases/tag/0.6.7

[oparoz]

Of course. Can me merge this and I then update all bower and composer packages?

[LukasReschke]

Yup. Let's do it that way.