Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0aa220b

Browse files
mheveryalxhub
authored andcommitted
fix(core): fix possible XSS attack in development through SSR. (#40136)
Escape the content of the strings so that it can be safely inserted into a comment node.The issue is that HTML does not specify any way to escape comment end text inside the comment.`<!-- The way you close a comment is with "-->". -->`. Above the `"-->"` is meant to be textnot an end to the comment. This can be created programmatically through DOM APIs.```div.innerHTML = div.innerHTML```One would expect that the above code would be safe to do, but it turns out that because commenttext is not escaped, the comment may contain text which will prematurely close the commentopening up the application for XSS attack. (In SSR we programmatically create comment nodes whichmay contain such text and expect them to be safe.)This function escapes the comment text by looking for the closing char sequence `-->` and replaceit with `-_-_>` where the `_` is a zero width space `\u200B`. The result is that if a commentcontains `-->` text it will render normally but it will not cause the HTML parser to close thecomment.PRClose#40136
1 parent03e8a92 commit0aa220b

File tree

6 files changed

+106
-4
lines changed

6 files changed

+106
-4
lines changed

‎packages/core/src/render3/instructions/shared.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {ViewEncapsulation} from '../../metadata/view';
1313
import{validateAgainstEventAttributes,validateAgainstEventProperties}from'../../sanitization/sanitization';
1414
import{Sanitizer}from'../../sanitization/sanitizer';
1515
import{assertDefined,assertDomNode,assertEqual,assertGreaterThanOrEqual,assertIndexInRange,assertNotEqual,assertNotSame,assertSame,assertString}from'../../util/assert';
16+
import{escapeCommentText}from'../../util/dom';
1617
import{createNamedArrayType}from'../../util/named_array_type';
1718
import{initNgDevMode}from'../../util/ng_dev_mode';
1819
import{normalizeDebugBindingName,normalizeDebugBindingValue}from'../../util/ng_reflect';
@@ -1043,7 +1044,8 @@ function setNgReflectProperty(
10431044
(elementasRElement).setAttribute(attrName,debugValue);
10441045
}
10451046
}else{
1046-
consttextContent=`bindings=${JSON.stringify({[attrName]:debugValue},null,2)}`;
1047+
consttextContent=
1048+
escapeCommentText(`bindings=${JSON.stringify({[attrName]:debugValue},null,2)}`);
10471049
if(isProceduralRenderer(renderer)){
10481050
renderer.setValue((elementasRComment),textContent);
10491051
}else{

‎packages/core/src/render3/node_manipulation.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {Renderer2} from '../render/api';
1111
import{RendererStyleFlags2}from'../render/api_flags';
1212
import{addToArray,removeFromArray}from'../util/array_utils';
1313
import{assertDefined,assertDomNode,assertEqual,assertFunction,assertString}from'../util/assert';
14+
import{escapeCommentText}from'../util/dom';
1415
import{assertLContainer,assertLView,assertTNodeForLView}from'./assert';
1516
import{attachPatchData}from'./context_discovery';
1617
import{icuContainerIterate}from'./i18n/i18n_tree_shaking';
@@ -113,7 +114,7 @@ export function createCommentNode(renderer: Renderer3, value: string): RComment
113114
ngDevMode&&ngDevMode.rendererCreateComment++;
114115
// isProceduralRenderer check is not needed because both `Renderer2` and `Renderer3` have the same
115116
// method name.
116-
returnrenderer.createComment(value);
117+
returnrenderer.createComment(escapeCommentText(value));
117118
}
118119

119120
/**

‎packages/core/src/util/dom.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/**
2+
*@license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
constEND_COMMENT=/-->/g;
10+
constEND_COMMENT_ESCAPED='-\u200B-\u200B>';
11+
12+
/**
13+
* Escape the content of the strings so that it can be safely inserted into a comment node.
14+
*
15+
* The issue is that HTML does not specify any way to escape comment end text inside the comment.
16+
* `<!-- The way you close a comment is with "-->". -->`. Above the `"-->"` is meant to be text not
17+
* an end to the comment. This can be created programmatically through DOM APIs.
18+
*
19+
* ```
20+
* div.innerHTML = div.innerHTML
21+
* ```
22+
*
23+
* One would expect that the above code would be safe to do, but it turns out that because comment
24+
* text is not escaped, the comment may contain text which will prematurely close the comment
25+
* opening up the application for XSS attack. (In SSR we programmatically create comment nodes which
26+
* may contain such text and expect them to be safe.)
27+
*
28+
* This function escapes the comment text by looking for the closing char sequence `-->` and replace
29+
* it with `-_-_>` where the `_` is a zero width space `\u200B`. The result is that if a comment
30+
* contains `-->` text it will render normally but it will not cause the HTML parser to close the
31+
* comment.
32+
*
33+
*@param value text to make safe for comment node by escaping the comment close character sequence
34+
*/
35+
exportfunctionescapeCommentText(value:string):string{
36+
returnvalue.replace(END_COMMENT,END_COMMENT_ESCAPED);
37+
}

‎packages/core/src/view/services.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {NgModuleRef} from '../linker/ng_module_factory';
1616
import{Renderer2,RendererFactory2}from'../render/api';
1717
import{RendererStyleFlags2,RendererType2}from'../render/api_flags';
1818
import{Sanitizer}from'../sanitization/sanitizer';
19+
import{escapeCommentText}from'../util/dom';
1920
import{isDevMode}from'../util/is_dev_mode';
2021
import{normalizeDebugBindingName,normalizeDebugBindingValue}from'../util/ng_reflect';
2122

@@ -448,7 +449,8 @@ function debugCheckAndUpdateNode(
448449
constel=asElementData(view,elDef.nodeIndex).renderElement;
449450
if(!elDef.element!.name){
450451
// a comment.
451-
view.renderer.setValue(el,`bindings=${JSON.stringify(bindingValues,null,2)}`);
452+
view.renderer.setValue(
453+
el,escapeCommentText(`bindings=${JSON.stringify(bindingValues,null,2)}`));
452454
}else{
453455
// a regular element.
454456
for(letattrinbindingValues){
@@ -727,7 +729,7 @@ export class DebugRenderer2 implements Renderer2 {
727729
}
728730

729731
createComment(value:string):any{
730-
constcomment=this.delegate.createComment(value);
732+
constcomment=this.delegate.createComment(escapeCommentText(value));
731733
constdebugCtx=this.createDebugContext(comment);
732734
if(debugCtx){
733735
indexDebugNode(newDebugNode__PRE_R3__(comment,null,debugCtx));
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/**
2+
*@license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import{Component}from'@angular/core';
10+
import{TestBed}from'@angular/core/testing';
11+
12+
13+
describe('comment node text escaping',()=>{
14+
it('should not be possible to do XSS through comment reflect data',()=>{
15+
@Component({template:`<div><span *ngIf="xssValue"></span><div>`})
16+
classXSSComp{
17+
xssValue:string='--> --><script>"evil"</script>';
18+
}
19+
20+
TestBed.configureTestingModule({declarations:[XSSComp]});
21+
constfixture=TestBed.createComponent(XSSComp);
22+
fixture.detectChanges();
23+
constdiv=fixture.nativeElement.querySelector('div')asHTMLElement;
24+
// Serialize into a string to mimic SSR serialization.
25+
consthtml=div.innerHTML;
26+
// This must be escaped or we have XSS.
27+
expect(html).not.toContain('--><script');
28+
// Now parse it back into DOM (from string)
29+
div.innerHTML=html;
30+
// Verify that we did not accidentally deserialize the `<script>`
31+
constscript=div.querySelector('script');
32+
expect(script).toBeFalsy();
33+
});
34+
});
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
*@license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import{escapeCommentText}from'@angular/core/src/util/dom';
10+
11+
describe('comment node text escaping',()=>{
12+
describe('escapeCommentText',()=>{
13+
it('should not change anything on basic text',()=>{
14+
expect(escapeCommentText('text')).toEqual('text');
15+
});
16+
17+
it('should escape end marker',()=>{
18+
expect(escapeCommentText('before-->after')).toEqual('before-\u200b-\u200b>after');
19+
});
20+
21+
it('should escape multiple markers',()=>{
22+
expect(escapeCommentText('before-->inline-->after'))
23+
.toEqual('before-\u200b-\u200b>inline-\u200b-\u200b>after');
24+
});
25+
});
26+
});

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp