Use Acorn.walk instead of the original crappy node walker

[pauljt]

WAIT WHAT IS THAT PANDA DOING ITS NOT EVEN MONDAY YET? [1]

Rewrite scanner to use Acorn.walk? Done! Change rules to not require eval? Done! Update experiment tab to use new scanner? Why not! Rewrite all the rules…. ummm ok there is still bit to do….

But I’m pumped. This approach means we can write rule templates (see [1]) which have the code, and then the rules themselves just contain parameters (“target”). I’m going to change it so you can supply complex parameters (i.e. JSON parameters). The workflow for changing rules is also really easy now.

Step 1: Modify this spreadsheet : Step 2: convert to a json file using http://shancarter.github.io/mr-data-converter/ Step 3: save it to a json file like rules.json

This has the added bonus of supporting dynamic loading of custom rules.

Man I am so pumped I need to watch [1] again.

[1] http://gifsound.com/?gif=i.imgur.com/xsKUETn.gif&v=9D-QD_HIfjA&s=18 [2] https://github.com/mozilla/scanjs/blob/newscanner/common/AcornWalker.js [3] https://docs.google.com/a/mozilla.com/spreadsheets/d/1T14mvMMAspnvhKXxPNRBiV0x6QKZsXr8_b7eXJvEf_A/edit?alt=json#gid=0

[pauljt]

PS unfortunately, even with the horrendous things I did with bind to avoid creating rule functions at runtime, acorn.js isn't compatible with CSP. Boourns. Thats a shame, because acorn.walk is freaking sweet, and I expect a lot less buggy that ScanJS.traverse...

[pauljt]

ps this work is in the newscanner branch, main changes in https://github.com/mozilla/scanjs/blob/newscanner/common/AcornWalker.js

[pwnetrationguru]

This looks awesome and I agree about preferring acorn.walk! I guess the obvious question is if CSP is still worth it?

We did it at first because it was easy and obviously something we recommend. Now, maybe a simple 'Don't Host this on Production' in the README is enough and we can just ditch CSP.

[pwnetrationguru]

fwiw, everything seems to work fine for me, but I haven't looked at the code much. I just ran tests and played with the client in Chrome in FF.

[pauljt]

Tests don’t use this scanner yet - I’m writing them at the moment. (need to extend rule templates, and then recreate all the rules)

And re:CSP, I wouldn’t remove it, its a nice to have. and there is no harm in leave the current relaxed policy, it just would be nice to get rid of unsafe-eval. I would like to see what acorn needs eval for - we might not even be using that functionality, in which case we can just modify the lib. The reason I would like to get a strict CSP policy is that I would like it to be safe for people to share rules. Now this doesn’t depend on CSP - the rules I have already for acorn no longer execute script, but it would be nice if it was provably safe, by use of a CSP.

On 7 Apr 2014, at 2:11 am, Rob Fletcher notifications@github.com wrote:

fwiw, everything seems to work fine for me, but I haven't looked at the code much. I just ran tests and played with the client in Chrome in FF.

— Reply to this email directly or view it on GitHub.

[defreez]

I would like to see what acorn needs eval for - we might not even be using that functionality, in which case we can just modify the lib.

Acorn uses Function to generate a big switch structure for string matching. It is definitely core functionality, but it could be patched out without too much difficulty by just creating the relevant functions instead of dynamically generating them.

marijnh/acorn#90 is an open issue for making Acorn CSP compliant, but marijnh doesn't seem super-excited about doing that.

[mozfreddyb]

fixed with #115