honeymoose/angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revert "fix(core): fix possible XSS attack in development through SSR (#40525)" (#40533)

Browse Source 
This reverts commit bb3b315eee8a669bf78940a24ecc4279cd04d380.

Reason for Revert: Issues with Google3 TAP Failures

PR Close #40533
This commit is contained in:
Jessica Janiuk 2021-01-22 16:37:17 -08:00
parent bb3b315eee
commit c64a56fbcc
3 changed files with 25 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
packages/core/src/util/dom.ts
View File

@ -6,18 +6,15 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
const END_COMMENT = /(<|>)/g; const END_COMMENT = /-->/g;
const END_COMMENT_ESCAPED = '\u200B$1\u200B'; const END_COMMENT_ESCAPED = '-\u200B-\u200B>';
/** /**
* Escape the content of the strings so that it can be safely inserted into a comment node. * 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 issue is that HTML does not specify any way to escape comment end text inside the comment.
* Consider: `<!-- The way you close a comment is with ">", and "->" at the beginning or by "-->" or * `<!-- The way you close a comment is with "-->". -->`. Above the `"-->"` is meant to be text not
* "--!>" at the end. -->`. Above the `"-->"` is meant to be text not an end to the comment. This * an end to the comment. This can be created programmatically through DOM APIs.
* can be created programmatically through DOM APIs. (`<!--` are also disallowed.)
*
* see: https://html.spec.whatwg.org/multipage/syntax.html#comments
* *
* ``` * ```
* div.innerHTML = div.innerHTML * div.innerHTML = div.innerHTML
@ -29,7 +26,7 @@ const END_COMMENT_ESCAPED = '\u200B$1\u200B';
* may contain such text and expect them to be safe.) * may contain such text and expect them to be safe.)
* *
* This function escapes the comment text by looking for the closing char sequence `-->` and replace * This function escapes the comment text by looking for the closing char sequence `-->` and replace
* it with `--_>_` where the `_` is a zero width space `\u200B`. The result is that if a comment * it with `-_-_>` where the `_` is a zero width space `\u200B`. The result is that if a comment
* contains `-->` text it will render normally but it will not cause the HTML parser to close the * contains `-->` text it will render normally but it will not cause the HTML parser to close the
* comment. * comment.
* *

45
packages/core/test/acceptance/security_spec.ts
View File

@ -11,33 +11,24 @@ import {TestBed} from '@angular/core/testing';
describe('comment node text escaping', () => { describe('comment node text escaping', () => {
// see: https://html.spec.whatwg.org/multipage/syntax.html#comments it('should not be possible to do XSS through comment reflect data', () => {
['>', // self closing @Component({template: `<div><span *ngIf="xssValue"></span><div>`})
'-->', // standard closing class XSSComp {
'--!>', // alternate closing xssValue: string = '--> --><script>"evil"</script>';
'<!-- -->', // embedded comment. }
].forEach((xssValue) => {
it('should not be possible to do XSS through comment reflect data when writing: ' + xssValue,
() => {
@Component({template: `<div><span *ngIf="xssValue"></span><div>`})
class XSSComp {
// ngIf serializes the `xssValue` into a comment for debugging purposes.
xssValue: string = xssValue + '<script>"evil"</script>';
}
TestBed.configureTestingModule({declarations: [XSSComp]}); TestBed.configureTestingModule({declarations: [XSSComp]});
const fixture = TestBed.createComponent(XSSComp); const fixture = TestBed.createComponent(XSSComp);
fixture.detectChanges(); fixture.detectChanges();
const div = fixture.nativeElement.querySelector('div') as HTMLElement; const div = fixture.nativeElement.querySelector('div') as HTMLElement;
// Serialize into a string to mimic SSR serialization. // Serialize into a string to mimic SSR serialization.
const html = div.innerHTML; const html = div.innerHTML;
// This must be escaped or we have XSS. // This must be escaped or we have XSS.
expect(html).not.toContain('--><script'); expect(html).not.toContain('--><script');
// Now parse it back into DOM (from string) // Now parse it back into DOM (from string)
div.innerHTML = html; div.innerHTML = html;
// Verify that we did not accidentally deserialize the `<script>` // Verify that we did not accidentally deserialize the `<script>`
const script = div.querySelector('script'); const script = div.querySelector('script');
expect(script).toBeFalsy(); expect(script).toBeFalsy();
});
}); });
}); });

11
packages/core/test/util/dom_spec.ts
View File

@ -14,20 +14,13 @@ describe('comment node text escaping', () => {
expect(escapeCommentText('text')).toEqual('text'); expect(escapeCommentText('text')).toEqual('text');
}); });
it('should escape "<" or ">"', () => {
expect(escapeCommentText('<')).toEqual('\u200b<\u200b');
expect(escapeCommentText('<<')).toEqual('\u200b<\u200b\u200b<\u200b');
expect(escapeCommentText('>')).toEqual('\u200b>\u200b');
expect(escapeCommentText('>>')).toEqual('\u200b>\u200b\u200b>\u200b');
});
it('should escape end marker', () => { it('should escape end marker', () => {
expect(escapeCommentText('before-->after')).toEqual('before--\u200b>\u200bafter'); expect(escapeCommentText('before-->after')).toEqual('before-\u200b-\u200b>after');
}); });
it('should escape multiple markers', () => { it('should escape multiple markers', () => {
expect(escapeCommentText('before-->inline-->after')) expect(escapeCommentText('before-->inline-->after'))
.toEqual('before--\u200b>\u200binline--\u200b>\u200bafter'); .toEqual('before-\u200b-\u200b>inline-\u200b-\u200b>after');
}); });
}); });
}); });