Skip to content
This repository has been archived by the owner on Jul 23, 2019. It is now read-only.

XSS due poor escaping/unescaping #61

Closed
WebReflection opened this issue Apr 8, 2015 · 2 comments
Closed

XSS due poor escaping/unescaping #61

WebReflection opened this issue Apr 8, 2015 · 2 comments
Labels

Comments

@WebReflection
Copy link

Somebody mentioned this library and I've noticed you folks are XSS prone.

This is a very bad approach to html escape/unescape, explained here why that is deadly wrong: https://gist.github.com/WebReflection/df05641bd04954f6d366

Thanks for any update and Best Regards

@WebReflection
Copy link
Author

OK, clarifying what you are doing wrong in that piece of highlighted code.

First of all that does not work in Chrome because third flag argument is not supported and it has been deprecated: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/replace#Parameters

See flags

Secondly, in browsers where that works you are vulnerable to JS injection:

ns.unescapeHTML('<script>alert("yo")</script>');

// will produce
// <script>alert("yo")</script>
// instead of expected
// &lt;script&gt;alert("yo")&lt;/script&gt;

The only way to escape/unescape a generic string is to grab all things that need to be escaped at once.

This is a little utility you can use in order to achieve the same in a secure way.

var html = (function () {
  // Andrea Giammarchi - WTFPL
  var
    reEscape = /[&<>'"]/g, // all at once
    reUnescape = /&(?:amp|#38|lt|#60|gt|#62|apos|#39|quot|#34);/g,
    oEscape = {
      '&': '&amp;',
      '<': '&lt;',
      '>': '&gt;',
      "'": '&#39;',
      '"': '&quot;'
    },
    oUnescape = {
      '&amp;': '&',
      '&#38;': '&',
      '&lt;': '<',
      '&#60;': '<',
      '&gt;': '>',
      '&#62;': '>',
      '&apos;': "'",
      '&#39;': "'",
      '&quot;': '"',
      '&#34;': '"'
    },
    fnEscape = function (m) {
      return oEscape[m];
    },
    fnUnescape = function (m) {
      return oUnescape[m];
    },
    replace = String.prototype.replace
  ;
  return (Object.freeze || Object)({
    escape: function escape(s) {
      return replace.call(s, reEscape, fnEscape);
    },
    unescape: function unescape(s) {
      return replace.call(s, reUnescape, fnUnescape);
    }
  });
}());

@mikebe11
Copy link
Contributor

Thanks for bringing that to our attention and for the detailed notes. This update should take care of it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants