Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make interrupt-checking function injection during js sanitzation more precise to avoid eval errors on complex scripts #26

Merged
merged 1 commit into from
Dec 29, 2017

Conversation

mellster2012
Copy link
Contributor

When processing complex scripts, e.g. containing browserified code, the sanitizer inserts teh interrupt checking function at wrong places and later fails to eval the secured js.
Example prefix:

var browserify_require=(function e(t,n,r){function s(o,u){if(!n[o]){if(!t[o]){var a=typeof require=="function"&&require;if(!u&&a)return a(o,!0);if(i)return i(o,!0);var f=new Error("Cannot find module '"+o+"'");throw f.code="MODULE_NOT_FOUND",f}var l=n[o]={exports:{}};t[o][0].call(l.exports,function(e){var n=t[o][1][e];return s(n?n:e)},l,l.exports,e,t,n,r)}return n[o].exports}var i=typeof require=="function"&&require;for(var o=0;o<r.length;o++)s(r[o]);return s})({1:[function(require,module,exports){

will insert __if(); at 2 wrong places:

  1. Due to matching on require=="function" (should only match on actual function)
  2. Due to matching on for(var o=0;o<r.length;o++)s(r[o]);return s})({ (erroneously interpreting the last curly brace as the start of the for block which is brace-less though)

This PR adjusts the regexes for the interrupt-checking function injection to be more selective and also include brace-less for-loops. Surely there is more to be done (e.g. the sanitizer currently also injects __if(); inside comments if the 10th semicolon happens to be inside a comment) but it's prudent to be defensive with altering regexes so this can be improved incrementally after having been tested thoroughly.
cheers

… precise to avoid eval errors on complex scripts
@javadelight javadelight merged commit 6773ed2 into javadelight:master Dec 29, 2017
@javadelight
Copy link
Owner

Looks good! Thank you!

Changed is included in newly released version 0.1.3.

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

Successfully merging this pull request may close these issues.

2 participants