Merge pull request #3816 from tanhauhau/tanhauhau/text-content-instead-of-inner-html

use textContent instead of innerHtml, preventing XSS

Author: Rich-Harris

src/compiler/compile/render_dom/wrappers/AwaitBlock.ts

@@ -69,6 +69,7 @@ export default class AwaitBlockWrapper extends Wrapper {
 		super(renderer, block, parent, node);
 
 		this.cannot_use_innerhtml();
+		this.not_static_content();
 
 		block.add_dependencies(this.node.expression.dependencies);
 

src/compiler/compile/render_dom/wrappers/EachBlock.ts

@@ -76,6 +76,7 @@ export default class EachBlockWrapper extends Wrapper {
 	) {
 		super(renderer, block, parent, node);
 		this.cannot_use_innerhtml();
+		this.not_static_content();
 
 		const { dependencies } = node.expression;
 		block.add_dependencies(dependencies);

src/compiler/compile/render_dom/wrappers/Element/Attribute.ts

@@ -19,6 +19,7 @@ export default class AttributeWrapper {
 
 		if (node.dependencies.size > 0) {
 			parent.cannot_use_innerhtml();
+			parent.not_static_content();
 
 			block.add_dependencies(node.dependencies);
 

src/compiler/compile/render_dom/wrappers/Element/index.ts

@@ -217,17 +217,17 @@ export default class ElementWrapper extends Wrapper {
 		});
 
 		if (this.parent) {
-			if (node.actions.length > 0) this.parent.cannot_use_innerhtml();
-			if (node.animation) this.parent.cannot_use_innerhtml();
-			if (node.bindings.length > 0) this.parent.cannot_use_innerhtml();
-			if (node.classes.length > 0) this.parent.cannot_use_innerhtml();
-			if (node.intro || node.outro) this.parent.cannot_use_innerhtml();
-			if (node.handlers.length > 0) this.parent.cannot_use_innerhtml();
-
-			if (this.node.name === 'option') this.parent.cannot_use_innerhtml();
-
-			if (renderer.options.dev) {
+			if (node.actions.length > 0 ||
+				node.animation ||
+				node.bindings.length > 0 ||
+				node.classes.length > 0 ||
+				node.intro || node.outro ||
+				node.handlers.length > 0 ||
+				this.node.name === 'option' ||
+				renderer.options.dev
+			) {
 				this.parent.cannot_use_innerhtml(); // need to use add_location
+				this.parent.not_static_content();
 			}
 		}
 
@@ -291,7 +291,7 @@ export default class ElementWrapper extends Wrapper {
 		}
 
 		// insert static children with textContent or innerHTML
-		if (!this.node.namespace && this.can_use_innerhtml && this.fragment.nodes.length > 0) {
+		if (!this.node.namespace && (this.can_use_innerhtml || this.can_use_textcontent()) && this.fragment.nodes.length > 0) {
 			if (this.fragment.nodes.length === 1 && this.fragment.nodes[0].node.type === 'Text') {
 				block.chunks.create.push(
 					 // @ts-ignore todo: should it be this.fragment.nodes[0].node.data instead?
@@ -315,7 +315,7 @@ export default class ElementWrapper extends Wrapper {
 				literal.quasis.push(state.quasi);
 
 				block.chunks.create.push(
-					b`${node}.innerHTML = ${literal};`
+					b`${node}.${this.can_use_innerhtml ? 'innerHTML': 'textContent'} = ${literal};`
 				);
 			}
 		} else {
@@ -361,6 +361,10 @@ export default class ElementWrapper extends Wrapper {
 		}
 	}
 
+	can_use_textcontent() {
+		return this.is_static_content && this.fragment.nodes.every(node => node.node.type === 'Text' || node.node.type === 'MustacheTag');
+	}
+
 	get_render_statement() {
 		const { name, namespace } = this.node;
 

src/compiler/compile/render_dom/wrappers/IfBlock.ts

@@ -95,6 +95,7 @@ export default class IfBlockWrapper extends Wrapper {
 		super(renderer, block, parent, node);
 
 		this.cannot_use_innerhtml();
+		this.not_static_content();
 
 		this.branches = [];
 

src/compiler/compile/render_dom/wrappers/InlineComponent/index.ts

@@ -34,6 +34,7 @@ export default class InlineComponentWrapper extends Wrapper {
 		super(renderer, block, parent, node);
 
 		this.cannot_use_innerhtml();
+		this.not_static_content();
 
 		if (this.node.expression) {
 			block.add_dependencies(this.node.expression.dependencies);

src/compiler/compile/render_dom/wrappers/RawMustacheTag.ts

@@ -19,6 +19,7 @@ export default class RawMustacheTagWrapper extends Tag {
 	) {
 		super(renderer, block, parent, node);
 		this.cannot_use_innerhtml();
+		this.not_static_content();
 	}
 
 	render(block: Block, parent_node: Identifier, _parent_nodes: Identifier) {

src/compiler/compile/render_dom/wrappers/Slot.ts

@@ -29,6 +29,7 @@ export default class SlotWrapper extends Wrapper {
 	) {
 		super(renderer, block, parent, node);
 		this.cannot_use_innerhtml();
+		this.not_static_content();
 
 		this.fragment = new FragmentWrapper(
 			renderer,

src/compiler/compile/render_dom/wrappers/shared/Tag.ts

@@ -11,8 +11,10 @@ export default class Tag extends Wrapper {
 
 	constructor(renderer: Renderer, block: Block, parent: Wrapper, node: MustacheTag | RawMustacheTag) {
 		super(renderer, block, parent, node);
+
+		this.cannot_use_innerhtml();
 		if (!this.is_dependencies_static()) {
-			this.cannot_use_innerhtml();
+			this.not_static_content();
 		}
 
 		block.add_dependencies(node.expression.dependencies);

src/compiler/compile/render_dom/wrappers/shared/Wrapper.ts

@@ -14,6 +14,7 @@ export default class Wrapper {
 
 	var: Identifier;
 	can_use_innerhtml: boolean;
+	is_static_content: boolean;
 
 	constructor(
 		renderer: Renderer,
@@ -35,6 +36,7 @@ export default class Wrapper {
 		});
 
 		this.can_use_innerhtml = !renderer.options.hydratable;
+		this.is_static_content = !renderer.options.hydratable;
 
 		block.wrappers.push(this);
 	}
@@ -44,6 +46,11 @@ export default class Wrapper {
 		if (this.parent) this.parent.cannot_use_innerhtml();
 	}
 
+	not_static_content() {
+		this.is_static_content = false;
+		if (this.parent) this.parent.not_static_content();
+	}
+
 	get_or_create_anchor(block: Block, parent_node: Identifier, parent_nodes: Identifier) {
 		// TODO use this in EachBlock and IfBlock — tricky because
 		// children need to be created first

test/js/samples/hoisted-const/expected.js

@@ -14,7 +14,7 @@ function create_fragment(ctx) {
 	return {
 		c() {
 			b = element("b");
-			b.innerHTML = `${get_answer()}`;
+			b.textContent = `${get_answer()}`;
 		},
 		m(target, anchor) {
 			insert(target, b, anchor);

test/js/samples/hoisted-let/expected.js

@@ -14,7 +14,7 @@ function create_fragment(ctx) {
 	return {
 		c() {
 			b = element("b");
-			b.innerHTML = `${get_answer()}`;
+			b.textContent = `${get_answer()}`;
 		},
 		m(target, anchor) {
 			insert(target, b, anchor);

test/js/samples/non-mutable-reference/expected.js

@@ -14,7 +14,7 @@ function create_fragment(ctx) {
 	return {
 		c() {
 			h1 = element("h1");
-			h1.innerHTML = `Hello ${name}!`;
+			h1.textContent = `Hello ${name}!`;
 		},
 		m(target, anchor) {
 			insert(target, h1, anchor);

test/js/samples/unchanged-expression/expected.js

@@ -14,6 +14,11 @@ import {
 
 function create_fragment(ctx) {
 	let div0;
+	let p0;
+	let t1;
+	let p1;
+	let t4;
+	let p2;
 	let t7;
 	let div1;
 	let p3;
@@ -23,11 +28,14 @@ function create_fragment(ctx) {
 	return {
 		c() {
 			div0 = element("div");
-
-			div0.innerHTML = `<p>Hello world</p> 
-  <p>Hello ${world1}</p> 
-  <p>Hello ${world2}</p>`;
-
+			p0 = element("p");
+			p0.textContent = "Hello world";
+			t1 = space();
+			p1 = element("p");
+			p1.textContent = `Hello ${world1}`;
+			t4 = space();
+			p2 = element("p");
+			p2.textContent = `Hello ${world2}`;
 			t7 = space();
 			div1 = element("div");
 			p3 = element("p");
@@ -36,6 +44,11 @@ function create_fragment(ctx) {
 		},
 		m(target, anchor) {
 			insert(target, div0, anchor);
+			append(div0, p0);
+			append(div0, t1);
+			append(div0, p1);
+			append(div0, t4);
+			append(div0, p2);
 			insert(target, t7, anchor);
 			insert(target, div1, anchor);
 			append(div1, p3);

test/runtime/samples/unchanged-expression-xss/_config.js

@@ -0,0 +1,3 @@
+export default {
+	html: `<p>&lt;b\nstyle='color:\nred;'&gt;RED?!?&lt;/b&gt;</p>`,
+};

test/runtime/samples/unchanged-expression-xss/main.svelte

@@ -0,0 +1,5 @@
+<script>
+  const content = `<b style='color: red;'>RED?!?</b>`
+</script>
+
+<p>{content}</p>