diff --git a/index.js b/index.js index f3db681..4d3e212 100644 --- a/index.js +++ b/index.js @@ -15,9 +15,16 @@ var IS_NATIVE_CODE_REGEXP = /\{\s*\[native code\]\s*\}/g; var IS_PURE_FUNCTION = /function.*?\(/; var IS_ARROW_FUNCTION = /.*?=>.*?/; var UNSAFE_CHARS_REGEXP = /[<>\/\u2028\u2029]/g; -// Regex to match and variations (case-insensitive) for XSS protection -// Matches -var SCRIPT_CLOSE_REGEXP = /<\/script[^>]*>/gi; +// Matches a script end tag (case-insensitive) for XSS protection: either a +// full `` tag, or a bare ``) that the HTML tokenizer +// treats as ending the tag name (see the WHATWG "script data end tag name +// state"). The bare-prefix form matters because the matching `>` could be +// supplied by a different serialized value later in the output, so escaping +// stops there without waiting for a closing `>`. A trailing backslash is not +// a delimiter here (that's a JS-level concern, not an HTML one), so this +// doesn't affect tagged template literals like `String.raw`. +var SCRIPT_CLOSE_REGEXP = /<\/script[^>]*>|<\/script(?=[\t\n\f\r \/>])/gi; var RESERVED_SYMBOLS = ['*', 'async']; @@ -35,16 +42,57 @@ function escapeUnsafeChars(unsafeChar) { return ESCAPED_CHARS[unsafeChar]; } -// Escape function body for XSS protection while preserving arrow function syntax +// Roughly matches string literals, template literals, regex literals, and +// comments, so `escapeFunctionBody` can treat their contents differently +// from plain code. This is a heuristic, not a full parser, with two known +// limitations: +// - A template literal is treated as one opaque span, including any +// `${...}` inside it. A `), comparison operators, and regex literals: only script end +// tags and line terminators are escaped. function escapeFunctionBody(str) { - // Escape sequences and variations (case-insensitive) - the main XSS risk - // Matches - // This must be done first before other replacements - str = str.replace(SCRIPT_CLOSE_REGEXP, function(match) { - // Escape all <, /, and > characters in the closing script tag - return match.replace(//g, '\\u003E'); + // Record the [start, end) span of every string/template/regex literal + // and comment, so a script-close match inside one can be escaped + // differently from a match in plain code (see below). + var stringAndCommentSpans = []; + var match; + STRING_OR_COMMENT_REGEXP.lastIndex = 0; + while ((match = STRING_OR_COMMENT_REGEXP.exec(str))) { + stringAndCommentSpans.push([match.index, match.index + match[0].length]); + } + + // Matches and spans are both in increasing offset order, so a single + // forward-moving cursor classifies every match in O(n) total instead of + // rescanning all spans for each match. + var spanCursor = 0; + + str = str.replace(SCRIPT_CLOSE_REGEXP, function(scriptCloseMatch, offset) { + while (spanCursor < stringAndCommentSpans.length && stringAndCommentSpans[spanCursor][1] <= offset) { + spanCursor++; + } + var span = stringAndCommentSpans[spanCursor]; + var inStringOrComment = !!span && offset >= span[0] && offset < span[1]; + if (!inStringOrComment) { + // In plain code, `<` and `/` may be real tokens (comparison, + // regex delimiter, division, ...), so they can't be rewritten + // as unicode escapes without breaking syntax. A space is a + // no-op here but still breaks up the `/g, '\\u003E'); }); - // Escape line terminators (these are always unsafe) str = str.replace(/\u2028/g, '\\u2028'); str = str.replace(/\u2029/g, '\\u2029'); return str; @@ -163,7 +211,6 @@ module.exports = function serialize(obj, options) { } // Escape unsafe HTML characters in function body for XSS protection - // This must preserve arrow function syntax (=>) while escaping if (options && options.unsafe !== true) { serializedFn = escapeFunctionBody(serializedFn); } diff --git a/test/unit/serialize.js b/test/unit/serialize.js index 2fc5709..978328f 100644 --- a/test/unit/serialize.js +++ b/test/unit/serialize.js @@ -666,6 +666,66 @@ describe('serialize( obj )', function () { strictEqual(typeof deserialized, 'function'); strictEqual(deserialized(), ''); }); + + it('should encode split script-closing payload across function bodies', function () { + var serialized = serialize({ + a: function () { /* */ } + }); + + strictEqual(serialized.includes(']/i.test(serialized), false); + + var deserialized; eval('deserialized = ' + serialized); + strictEqual(deserialized("'"), fn("'")); + strictEqual(deserialized('x'), fn('x')); + }); + + it('should not let a `/` inside a regex character class end the regex literal early', function () { + // `/` inside `[...]` doesn't need to be escaped and doesn't + // terminate the regex literal. If it were mistaken for the + // closing delimiter, the real string literal that follows would + // be misidentified and its `]/i.test(serialized), false); + + var deserialized; eval('deserialized = ' + serialized); + strictEqual(deserialized("'"), fn("'")); + strictEqual(deserialized('/'), fn('/')); + strictEqual(deserialized('x'), fn('x')); + }); }); describe('options', function () {