Notification's HTML rendering disabled by default

@cgvwzq commented on Aug 22, 2018, 10:19 AM UTC:

Summary

HTML rendering in notification's messages should be disabled by default, and only done when an explicit option's parameter ("html":true?) is passed.

Motivation

Most of the time notifications only require displaying plain text, so accepting HTML by default is increasing the attack surface unnecessary. Notifications are a big source of HTML injection issues (user controlled values end up being displayed there).

Even if using DOMPurify makes the problem less severe, with its default configuration <style> tags are still a risk, and together with a relaxed CSP policy constitute a severe security issue.

Disabling HTML rendering would easily reduce HTML injection vulnerabilities.

Describe alternatives you've considered

First alternative would be to add <style> tag to the FORBID_TAGS list in DOMPurify. Since this can break some use cases, is probably better to use a secure-by-default alternative where notifications only accept plain text unless explicitly requested.

Additional context

Stylesheets are powerful enough to leak arbitrary content from the DOM. We should be treating them with the same respect than cross-site scripting :)

This issue was moved by rsese from atom/atom#17897.

@Aerijo commented on Aug 22, 2018, 10:23 AM UTC:

I wish they supported HTML and markdown even more, not scale it back.

@cgvwzq commented on Aug 22, 2018, 11:30 AM UTC:

You can have as much support as you want, but with great powers come great responsibilities. You should make the risks explicit and make developers apply the right countermeasures :) Simply adding a flag in order to allow "dangerous HTML" reduce risks, and still allows you to do as much as you want, but being more aware.

[cgvwzq]

Hey, any more thoughts on this? :)