Skip to content

Commit 4571561

Browse files
authored
fix(icon): remove svgSrc, only allow trusted urls (#1933)
* fix(icon): remove svgSrc, only allow trusted urls * rxjs
1 parent 221c234 commit 4571561

File tree

6 files changed

+90
-146
lines changed

6 files changed

+90
-146
lines changed

src/demo-app/icon/icon-demo.html

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,6 @@
33
These are some icons.
44
</p>
55

6-
<p>
7-
From URL:
8-
<md-icon svgSrc="/icon/assets/search-icon.svg"></md-icon>
9-
</p>
10-
116
<p>
127
By name registered with MdIconProvider:
138
<md-icon svgIcon="thumb-up" class="green"></md-icon>
@@ -37,4 +32,4 @@
3732
Custom icon font and CSS:
3833
<md-icon fontSet="fontawesome" fontIcon="fa-birthday-cake"></md-icon>
3934
</p>
40-
</div>
35+
</div>

src/demo-app/icon/icon-demo.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {Component, ViewEncapsulation} from '@angular/core';
2+
import {DomSanitizer} from '@angular/platform-browser';
23
import {MdIconRegistry} from '@angular/material';
34

45
@Component({
@@ -10,10 +11,12 @@ import {MdIconRegistry} from '@angular/material';
1011
encapsulation: ViewEncapsulation.None,
1112
})
1213
export class IconDemo {
13-
constructor(mdIconRegistry: MdIconRegistry) {
14+
constructor(mdIconRegistry: MdIconRegistry, sanitizer: DomSanitizer) {
1415
mdIconRegistry
15-
.addSvgIcon('thumb-up', '/icon/assets/thumbup-icon.svg')
16-
.addSvgIconSetInNamespace('core', '/icon/assets/core-icon-set.svg')
16+
.addSvgIcon('thumb-up',
17+
sanitizer.bypassSecurityTrustResourceUrl('/icon/assets/thumbup-icon.svg'))
18+
.addSvgIconSetInNamespace('core',
19+
sanitizer.bypassSecurityTrustResourceUrl('/icon/assets/core-icon-set.svg'))
1720
.registerFontClassAlias('fontawesome', 'fa');
1821
}
1922
}

src/lib/icon/icon-registry.ts

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1-
import {Injectable} from '@angular/core';
1+
import {Injectable, SecurityContext} from '@angular/core';
2+
import {SafeResourceUrl, DomSanitizer} from '@angular/platform-browser';
23
import {Http} from '@angular/http';
34
import {MdError} from '../core';
45
import {Observable} from 'rxjs/Observable';
56
import 'rxjs/add/observable/forkJoin';
67
import 'rxjs/add/observable/of';
8+
import 'rxjs/add/observable/throw';
79
import 'rxjs/add/operator/map';
810
import 'rxjs/add/operator/filter';
911
import 'rxjs/add/operator/do';
@@ -18,7 +20,7 @@ import 'rxjs/add/operator/catch';
1820
*/
1921
export class MdIconNameNotFoundError extends MdError {
2022
constructor(iconName: string) {
21-
super(`Unable to find icon with the name "${iconName}"`);
23+
super(`Unable to find icon with the name "${iconName}"`);
2224
}
2325
}
2426

@@ -29,7 +31,7 @@ export class MdIconNameNotFoundError extends MdError {
2931
*/
3032
export class MdIconSvgTagNotFoundError extends MdError {
3133
constructor() {
32-
super('<svg> tag not found');
34+
super('<svg> tag not found');
3335
}
3436
}
3537

@@ -39,7 +41,7 @@ export class MdIconSvgTagNotFoundError extends MdError {
3941
*/
4042
class SvgIconConfig {
4143
svgElement: SVGElement = null;
42-
constructor(public url: string) { }
44+
constructor(public url: SafeResourceUrl) { }
4345
}
4446

4547
/** Returns the cache key to use for an icon namespace and name. */
@@ -81,27 +83,27 @@ export class MdIconRegistry {
8183
*/
8284
private _defaultFontSetClass = 'material-icons';
8385

84-
constructor(private _http: Http) {}
86+
constructor(private _http: Http, private _sanitizer: DomSanitizer) {}
8587

8688
/** Registers an icon by URL in the default namespace. */
87-
addSvgIcon(iconName: string, url: string): this {
89+
addSvgIcon(iconName: string, url: SafeResourceUrl): this {
8890
return this.addSvgIconInNamespace('', iconName, url);
8991
}
9092

9193
/** Registers an icon by URL in the specified namespace. */
92-
addSvgIconInNamespace(namespace: string, iconName: string, url: string): this {
94+
addSvgIconInNamespace(namespace: string, iconName: string, url: SafeResourceUrl): this {
9395
const key = iconKey(namespace, iconName);
9496
this._svgIconConfigs.set(key, new SvgIconConfig(url));
9597
return this;
9698
}
9799

98100
/** Registers an icon set by URL in the default namespace. */
99-
addSvgIconSet(url: string): this {
101+
addSvgIconSet(url: SafeResourceUrl): this {
100102
return this.addSvgIconSetInNamespace('', url);
101103
}
102104

103105
/** Registers an icon set by URL in the specified namespace. */
104-
addSvgIconSetInNamespace(namespace: string, url: string): this {
106+
addSvgIconSetInNamespace(namespace: string, url: SafeResourceUrl): this {
105107
const config = new SvgIconConfig(url);
106108
if (this._iconSetConfigs.has(namespace)) {
107109
this._iconSetConfigs.get(namespace).push(config);
@@ -152,7 +154,9 @@ export class MdIconRegistry {
152154
* the produced element will always be a new copy of the originally fetched icon. (That is,
153155
* it will not contain any modifications made to elements previously returned).
154156
*/
155-
getSvgIconFromUrl(url: string): Observable<SVGElement> {
157+
getSvgIconFromUrl(safeUrl: SafeResourceUrl): Observable<SVGElement> {
158+
let url = this._sanitizer.sanitize(SecurityContext.RESOURCE_URL, safeUrl);
159+
156160
if (this._cachedIconsByUrl.has(url)) {
157161
return Observable.of(cloneSvg(this._cachedIconsByUrl.get(url)));
158162
}
@@ -221,9 +225,12 @@ export class MdIconRegistry {
221225
.map(iconSetConfig =>
222226
this._loadSvgIconSetFromConfig(iconSetConfig)
223227
.catch((err: any, caught: Observable<SVGElement>): Observable<SVGElement> => {
228+
let url =
229+
this._sanitizer.sanitize(SecurityContext.RESOURCE_URL, iconSetConfig.url);
230+
224231
// Swallow errors fetching individual URLs so the combined Observable won't
225232
// necessarily fail.
226-
console.log(`Loading icon set URL: ${iconSetConfig.url} failed: ${err}`);
233+
console.log(`Loading icon set URL: ${url} failed: ${err}`);
227234
return Observable.of(null);
228235
})
229236
.do(svg => {
@@ -280,7 +287,7 @@ export class MdIconRegistry {
280287
private _loadSvgIconSetFromConfig(config: SvgIconConfig): Observable<SVGElement> {
281288
// TODO: Document that icons should only be loaded from trusted sources.
282289
return this._fetchUrl(config.url)
283-
.map((svgText) => this._svgElementFromString(svgText));
290+
.map(svgText => this._svgElementFromString(svgText));
284291
}
285292

286293
/**
@@ -353,7 +360,9 @@ export class MdIconRegistry {
353360
* Returns an Observable which produces the string contents of the given URL. Results may be
354361
* cached, so future calls with the same URL may not cause another HTTP request.
355362
*/
356-
private _fetchUrl(url: string): Observable<string> {
363+
private _fetchUrl(safeUrl: SafeResourceUrl): Observable<string> {
364+
let url = this._sanitizer.sanitize(SecurityContext.RESOURCE_URL, safeUrl);
365+
357366
// Store in-progress fetches to avoid sending a duplicate request for a URL when there is
358367
// already a request in progress for that URL. It's necessary to call share() on the
359368
// Observable returned by http.get() so that multiple subscribers don't cause multiple XHRs.

0 commit comments

Comments
 (0)