Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

Commit 9b9059a

Browse files
devversionhansl
devversion
authored and
hansl
committed
fix(autocomplete) highlight directive should transform regex text into html entities. (#8368)
* The highlight controller no longer sanitizes the term or content, because this would require complex libraries like `ngSanitize` * Also there is no need to sanitize those strings, because after this change we never insert them as HTML nodes anymore. * We decompose the content string into different tokens and then compose them together into different elements. This allows us to be 100% sure that we only insert trusted HTML code and no unsafe HTML code, which could include XSS attacks. * This approach also supports now HTML identifiers and special text characters in the highlight text and content. Fixes #8356
1 parent 7c4b434 commit 9b9059a

File tree

3 files changed

+204
-35
lines changed

3 files changed

+204
-35
lines changed

‎src/components/autocomplete/autocomplete.spec.js

Lines changed: 94 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1280,9 +1280,11 @@ describe('<md-autocomplete>', function() {
12801280
});
12811281

12821282
describe('xss prevention', function() {
1283+
12831284
it('should not allow html to slip through', inject(function($timeout, $material) {
12841285
var html = 'foo <img src="img" onerror="alert(1)" />';
1285-
var scope = createScope([{display: html}]);
1286+
var scope = createScope([{ display: html }]);
1287+
12861288
var template = '\
12871289
<md-autocomplete\
12881290
md-selected-item="selectedItem"\
@@ -1293,6 +1295,7 @@ describe('<md-autocomplete>', function() {
12931295
placeholder="placeholder">\
12941296
<span md-highlight-text="searchText">{{item.display}}</span>\
12951297
</md-autocomplete>';
1298+
12961299
var element = compile(template, scope);
12971300
var ctrl = element.controller('mdAutocomplete');
12981301
var ul = element.find('ul');
@@ -1316,6 +1319,7 @@ describe('<md-autocomplete>', function() {
13161319

13171320
element.remove();
13181321
}));
1322+
13191323
});
13201324

13211325
describe('Async matching', function() {
@@ -1840,6 +1844,7 @@ describe('<md-autocomplete>', function() {
18401844
});
18411845

18421846
describe('md-highlight-text', function() {
1847+
18431848
it('updates when content is modified', inject(function() {
18441849
var template = '<div md-highlight-text="query">{{message}}</div>';
18451850
var scope = createScope(null, {message: 'some text', query: 'some'});
@@ -1860,7 +1865,7 @@ describe('<md-autocomplete>', function() {
18601865
element.remove();
18611866
}));
18621867

1863-
it('should properly apply highlight flags', inject(function() {
1868+
it('should properly apply highlight flags', function() {
18641869
var template = '<div md-highlight-text="query" md-highlight-flags="{{flags}}">{{message}}</div>';
18651870
var scope = createScope(null, {message: 'Some text', query: 'some', flags: '^i'});
18661871
var element = compile(template, scope);
@@ -1892,7 +1897,93 @@ describe('<md-autocomplete>', function() {
18921897
expect(element.html()).toBe('Some text, some flag<span class="highlight">s</span>');
18931898

18941899
element.remove();
1895-
}));
1900+
});
1901+
1902+
it('should correctly parse special text identifiers', function() {
1903+
var template = '<div md-highlight-text="query">{{message}}</div>';
1904+
1905+
var scope = createScope(null, {
1906+
message: 'Angular&Material',
1907+
query: 'Angular&'
1908+
});
1909+
1910+
var element = compile(template, scope);
1911+
1912+
expect(element.html()).toBe('<span class="highlight">Angular&amp;</span>Material');
1913+
1914+
scope.query = 'Angular&Material';
1915+
scope.$apply();
1916+
1917+
expect(element.html()).toBe('<span class="highlight">Angular&amp;Material</span>');
1918+
1919+
element.remove();
1920+
});
1921+
1922+
it('should properly parse html entity identifiers', function() {
1923+
var template = '<div md-highlight-text="query">{{message}}</div>';
1924+
1925+
var scope = createScope(null, {
1926+
message: 'Angular&amp;Material',
1927+
query: ''
1928+
});
1929+
1930+
var element = compile(template, scope);
1931+
1932+
expect(element.html()).toBe('Angular&amp;amp;Material');
1933+
1934+
scope.query = 'Angular&amp;Material';
1935+
scope.$apply();
1936+
1937+
expect(element.html()).toBe('<span class="highlight">Angular&amp;amp;Material</span>');
1938+
1939+
1940+
scope.query = 'Angular&';
1941+
scope.$apply();
1942+
1943+
expect(element.html()).toBe('<span class="highlight">Angular&amp;</span>amp;Material');
1944+
1945+
element.remove();
1946+
});
1947+
1948+
it('should prevent XSS attacks from the highlight text', function() {
1949+
1950+
spyOn(window, 'alert');
1951+
1952+
var template = '<div md-highlight-text="query">{{message}}</div>';
1953+
1954+
var scope = createScope(null, {
1955+
message: 'Angular Material',
1956+
query: '<img src="img" onerror="alert(1)">'
1957+
});
1958+
1959+
var element = compile(template, scope);
1960+
1961+
expect(element.html()).toBe('Angular Material');
1962+
expect(window.alert).not.toHaveBeenCalled();
1963+
1964+
element.remove();
1965+
});
1966+
1967+
});
1968+
1969+
it('should prevent XSS attacks from the content text', function() {
1970+
1971+
spyOn(window, 'alert');
1972+
1973+
var template = '<div md-highlight-text="query">{{message}}</div>';
1974+
1975+
var scope = createScope(null, {
1976+
message: '<img src="img" onerror="alert(1)">',
1977+
query: ''
1978+
});
1979+
1980+
var element = compile(template, scope);
1981+
1982+
// Expect the image to be escaped due to XSS protection.
1983+
expect(element.html()).toBe('&lt;img src="img" onerror="alert(1)"&gt;');
1984+
expect(window.alert).not.toHaveBeenCalled();
1985+
1986+
element.remove();
18961987
});
18971988

18981989
});

‎src/components/autocomplete/js/highlightController.js

Lines changed: 108 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,38 +3,116 @@ angular
33
.controller('MdHighlightCtrl', MdHighlightCtrl);
44

55
function MdHighlightCtrl ($scope, $element, $attrs) {
6-
this.init = init;
7-
8-
function init (termExpr, unsafeTextExpr) {
9-
var text = null,
10-
regex = null,
11-
flags = $attrs.mdHighlightFlags || '',
12-
watcher = $scope.$watch(function($scope) {
13-
return {
14-
term: termExpr($scope),
15-
unsafeText: unsafeTextExpr($scope)
16-
};
17-
}, function (state, prevState) {
18-
if (text === null || state.unsafeText !== prevState.unsafeText) {
19-
text = angular.element('<div>').text(state.unsafeText).html();
20-
}
21-
if (regex === null || state.term !== prevState.term) {
22-
regex = getRegExp(state.term, flags);
23-
}
24-
25-
$element.html(text.replace(regex, '<span class="highlight">$&</span>'));
26-
}, true);
27-
$element.on('$destroy', watcher);
6+
this.$scope = $scope;
7+
this.$element = $element;
8+
this.$attrs = $attrs;
9+
10+
// Cache the Regex to avoid rebuilding each time.
11+
this.regex = null;
12+
}
13+
14+
MdHighlightCtrl.prototype.init = function(unsafeTermFn, unsafeContentFn) {
15+
16+
this.flags = this.$attrs.mdHighlightFlags || '';
17+
18+
this.unregisterFn = this.$scope.$watch(function($scope) {
19+
return {
20+
term: unsafeTermFn($scope),
21+
contentText: unsafeContentFn($scope)
22+
};
23+
}.bind(this), this.onRender.bind(this), true);
24+
25+
this.$element.on('$destroy', this.unregisterFn);
26+
};
27+
28+
/**
29+
* Triggered once a new change has been recognized and the highlighted
30+
* text needs to be updated.
31+
*/
32+
MdHighlightCtrl.prototype.onRender = function(state, prevState) {
33+
34+
var contentText = state.contentText;
35+
36+
/* Update the regex if it's outdated, because we don't want to rebuilt it constantly. */
37+
if (this.regex === null || state.term !== prevState.term) {
38+
this.regex = this.createRegex(state.term, this.flags);
2839
}
2940

30-
function sanitize (term) {
31-
return term && term.toString().replace(/[\\\^\$\*\+\?\.\(\)\|\{}\[\]]/g, '\\$&');
41+
/* If a term is available apply the regex to the content */
42+
if (state.term) {
43+
this.applyRegex(contentText);
44+
} else {
45+
this.$element.text(contentText);
3246
}
3347

34-
function getRegExp (text, flags) {
35-
var startFlag = '', endFlag = '';
36-
if (flags.indexOf('^') >= 0) startFlag = '^';
37-
if (flags.indexOf('$') >= 0) endFlag = '$';
38-
return new RegExp(startFlag + sanitize(text) + endFlag, flags.replace(/[\$\^]/g, ''));
48+
};
49+
50+
/**
51+
* Decomposes the specified text into different tokens (whether match or not).
52+
* Breaking down the string guarantees proper XSS protection due to the native browser
53+
* escaping of unsafe text.
54+
*/
55+
MdHighlightCtrl.prototype.applyRegex = function(text) {
56+
var tokens = this.resolveTokens(text);
57+
58+
this.$element.empty();
59+
60+
tokens.forEach(function (token) {
61+
62+
if (token.isMatch) {
63+
var tokenEl = angular.element('<span class="highlight">').text(token.text);
64+
65+
this.$element.append(tokenEl);
66+
} else {
67+
this.$element.append(document.createTextNode(token));
68+
}
69+
70+
}.bind(this));
71+
72+
};
73+
74+
/**
75+
* Decomposes the specified text into different tokens by running the regex against the text.
76+
*/
77+
MdHighlightCtrl.prototype.resolveTokens = function(string) {
78+
var tokens = [];
79+
var lastIndex = 0;
80+
81+
// Use replace here, because it supports global and single regular expressions at same time.
82+
string.replace(this.regex, function(match, index) {
83+
appendToken(lastIndex, index);
84+
85+
tokens.push({
86+
text: match,
87+
isMatch: true
88+
});
89+
90+
lastIndex = index + match.length;
91+
});
92+
93+
// Append the missing text as a token.
94+
appendToken(lastIndex);
95+
96+
return tokens;
97+
98+
function appendToken(from, to) {
99+
var targetText = string.slice(from, to);
100+
targetText && tokens.push(targetText);
39101
}
40-
}
102+
};
103+
104+
/** Creates a regex for the specified text with the given flags. */
105+
MdHighlightCtrl.prototype.createRegex = function(term, flags) {
106+
var startFlag = '', endFlag = '';
107+
var regexTerm = this.sanitizeRegex(term);
108+
109+
if (flags.indexOf('^') >= 0) startFlag = '^';
110+
if (flags.indexOf('$') >= 0) endFlag = '$';
111+
112+
return new RegExp(startFlag + regexTerm + endFlag, flags.replace(/[$\^]/g, ''));
113+
};
114+
115+
/** Sanitizes a regex by removing all common RegExp identifiers */
116+
MdHighlightCtrl.prototype.sanitizeRegex = function(term) {
117+
return term && term.toString().replace(/[\\\^\$\*\+\?\.\(\)\|\{}\[\]]/g, '\\$&');
118+
};

‎src/components/autocomplete/js/highlightDirective.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@ function MdHighlight ($interpolate, $parse) {
3737
controller: 'MdHighlightCtrl',
3838
compile: function mdHighlightCompile(tElement, tAttr) {
3939
var termExpr = $parse(tAttr.mdHighlightText);
40-
var unsafeTextExpr = $interpolate(tElement.html());
40+
var unsafeContentExpr = $interpolate(tElement.html());
4141

4242
return function mdHighlightLink(scope, element, attr, ctrl) {
43-
ctrl.init(termExpr, unsafeTextExpr);
43+
ctrl.init(termExpr, unsafeContentExpr);
4444
};
4545
}
4646
};

0 commit comments

Comments
 (0)