fix(autocomplete): properly parse unsafe strings to prevent advanced XSS attacks

[devversion]

• 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

[robertmesserle]

@ThomasBurleson LGTM, closes #8388 as well.

[jammed343]

This doesn't work properly with html escaping. Also Is there a reason why you are using html escaping instead of regex escaping.

I changed:

function parseUnsafeString(text) {
    return angular.element('<div>').text(text).html();
}

To:

function parseUnsafeString(text) {
    // http://stackoverflow.com/questions/3446170/escape-string-for-use-in-javascript-regex
    return text.replace(/[\-\[\]\/\{\}\(\)\*\+\?\.\\\^\$\|]/g, "\\$&");
 }

Which makes it work well for me.

Otherwise i can search for "amp" and all the ampersands come up in search results with the current solution.

I suggest we add regex escape and then merge this into angular-material

[devversion]

@jammed343 I cannot see that issue. The way we currently parse the unsafe string, seems to work as expected. It just depends on the native browser behavior, which makes sure that the displayed results and the search text match.

Can you explain what issues you are experiencing? Did you run that PR already?

[jammed343]

@devversion
Using your current html escaping gives me this:

this last screenshot is just to show that the data is fine normally

[devversion]

@jammed343 Everything works for me as expected. And the tests are also passing.

How did you pull / download this PR to run on your codebase?

Before	After

Also the way this Regex parses the string is not valid. We want to retrieve the HTML encoded way, instead of the UTF-8 text.

Here is a demo: http://codepen.io/DevVersion/pen/PzrQGy

[jammed343]

@devversion I used the DevVersion:fix/parse-unsafe-regex branch which didn't work for me at first. then i modified the parseUnsafeString function you added to use regex escape and that worked for me.

[devversion]

@jammed343 Hmm weird. The tests pass, the demos are working as expected. It may be an issue on the PR pulling. Not sure how you served your application with the new changes etc.

Also as seen in my demo about the parsing, it is expected to turn & into & so we can properly match them.

[jammed343]

@devversion

I added some console output to your original pull:

if (text === null || state.unsafeText !== prevState.unsafeText) {
5        text = parseUnsafeString(state.unsafeText);
5      }
-      if (regex === null || state.term !== prevState.term) {
5        regex = getRegExp(parseUnsafeString(state.term), flags);
5      }
Added these console.logs
4      console.log("STATE CHANGE");
4      console.log("state.unsafeText: ", state.unsafeText);
4      console.log("state.term: ", state.term);
4      console.log("text: ", parseUnsafeString(state.unsafeText));
4      console.log("regex: ", getRegExp(parseUnsafeString(state.term), flags));
4      console.log("$element.html", text.replace(regex, '<span class="highlight">$&</span>'));
4
4      $element.html(text.replace(regex, '<span class="highlight">$&</span>'));

STATE CHANGE
angular-material.js:23902 state.unsafeText:  Animals & Pet Supplies > Pet Supplies > Bird Supplies > Bird Cages & Stands
angular-material.js:23903 state.term:  a
angular-material.js:23904 text:  Animals & Pet Supplies > Pet Supplies > Bird Supplies > Bird Cages & Stands
angular-material.js:23905 regex:  /a/gi
angular-material.js:23906 $element.html <span class="highlight">A</span>nim<span class="highlight">a</span>ls &<span class="highlight">a</span>mp; Pet Supplies &gt; Pet Supplies &gt; Bird Supplies &gt; Bird C<span class="highlight">a</span>ges &<span class="highlight">a</span>mp; St<span class="highlight">a</span>nds

[jammed343]

@devversion
Your latest commit:
776d82a
is now working.

[devversion]

@ThomasBurleson @jammed343 I made some changes regarding the way we display the highlighted text.

Those changes were necessary to support HTML entity identifiers and special characters inside of the highlight text.

The current approach was very unstable and really dangerous for XSS attacks, since we are setting the HTML of the element.

[ThomasBurleson]

@jammed343 - Thx for your persistence on improving this PR. 👍
@devversion - lgtm.

[ThomasBurleson]

@devversion - plz squash and (if needed) rebase.