Coder Social home page Coder Social logo

scanjs's Introduction

Development of ScanJS has stopped.

We are using ESLint instead. Your options:

  • XSS-prevention rule during CI with eslint-plugin-no-unsanitized. This assumes that you prevent/disallow obfuscation and that your code is subject to code review before it is being scanned (e.g., to catch intentional malicious/obfuscated code)
  • If you want something that warn for "all the things" that scanjs pointed out, you will need to use and build upon eslint-config-scanjs.

Thank you.


ScanJS was was a Static analysis tool for javascript code. ScanJS was created as an aid for security review, to help identify security issues in client-side web applications.

ScanJS used Acorn to convert sources to AST, then walks AST looking for source patterns. You could use the rules file supplied, or load your own rules.

ScanJS Rules

Rules are specified in JSON format - for an example see /common/template_rules.json

At a minimum, each must have rule is made up of 2 attributes:

  • name: the name of the rule
  • source: javascript source which matches one of the patterns below (see Rule Syntax below)

Optionally a rule may have the following attirbutes:

  • testhit: one more JavaScript statements (seperate by semi-colons) that the rule will match
  • testmiss: the rule should not match any of these statements
  • desc: description of the rule
  • threat: for catgorizing rules by threat

Rule Syntax

For the source attribute, the following basic statements are supported:

  • identifier foo: matches any identifier , "foo"
  • property $_any.foo: $_any is wildcard, matches anything.foo
  • objectproperty foo.bar: matches object and property, i.e. foo.bar

You can also matches function calls based on the same syntax:

  • call foo(): matches function calls with this name
  • propertycall $_any.foo: matches anything.foo() but not foo()
  • objectpropertycall: foo.bar(): matches foo.bar() only

You can also search for functions with matching literal arguments:

  • callargs foo('test',ignored,42): matches a function called foo, with 'test' as the first argument, anything as the second argument, and the number 42 as the third argument (i.e. matches ONLY literal arguments).
  • propertycallargs $_any.foo('test',ignored,42): same as above, but function has to be a property.
  • objectpropertycallargs foo.bar('test',ignored,42): same as above, but matches both object and property

You can also search for assignment to a specifically named identifier:

  • assignment foo=$_any: matches when foo is assigned to something
  • propertyassignment $_any.foo=$_any: matches when anything.foo is assigned to something
  • objectpropertyassignment foo.bar=$_any: matches when foo.bar is assigned to something

If you specify $_unsafe on the right hand side (e.g. foo.innerHTML=$_unsafe), it will only match if the RHS contains at least one identifier.

Tips:

  • Javascript is very dynamic, and this is navie approach: write conservative rules and review for false positives
  • One simple statement per rule, not complex statements (yet)!
  • 'foo' does NOT match 'this.foo', if you are looking for something in global (e.g. 'alert()' ), you need to add two rules: 'alert.()' and '$_any.alert()'
  • Try the rule out in the experiment tab to test what it matches

Examples: See /common/template_rules.json and /common/rules.json

Running ScanJS

Run ScanJS in the browser

Run ScanJS from the command line

  • Install node.js
  • scanner.js -t DIRECTORY_PATH

Testing instructions

Tests use the mocha testing framework.

  • npm test
  • or in the browser:http://127.0.0.1:4000/tests/

Tests are included in the rules declaration (see common/rules.json) by specifying the following two attributes, which are specified in the form of a series of javascript statements:

  • testhit: The rule should match each of these statements individualy.
  • testmiss: The rule should not match all of these statements.

scanjs's People

Contributors

arroway avatar callicles avatar codelion avatar dbkaplun avatar defreez avatar dpnishant avatar mozfreddyb avatar nmaier avatar zombie avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

scanjs's Issues

rearrange the angular controllers (this applies to scanctrl mostly)

For now we have a Controller for the rules and the Scanning parts, but the Scanning thing is already not really for scanning but for file uploads and other things.

Given that the web ui might become more separated and the scanning might once become async (if we use a Worker, see #7) the AngularJS Controller should probably be rearranged.

Also, AngularJS currently doesnt really handle file uploads, so there's already a workaround. But I think I'd lean towards keeping AngularJS as the framework for now

[testing][false negative] - var b = alert; b(1);

In general, we miss a case I've been calling 'variable indirection'.

If we use the example in the title of this issue, var b = alert; b(1);. We may wish to catch calls to alert(), but right now ScanJS misses the above case.

Failures:

var a = crypto; a.generate = crypto.generateCRMFRequest; a.generate("CN=0", 0, 0, null, "console.log(1)", 384, null, "rsa-dual-use");
var a = window.document; a.b = document.writeln; a.b("<h1>bad</h1>");
var a = window.document; a.b = document.writeln; a.b("<h1>bad</h1>");
var a = eval; a("alert(0);");
var a = Function; new a("alert(0)")();
var a = window.setInterval; a("console.log(4)", 300);
var a = window.setTimeout; a("console.log(4)", 300);
var o = window.open; o("http://www.mozilla.org", "name", {});

The following rules will need to be adjusted:

  • generateCRMFrequest:
  • document.write
  • document.writeln
  • eval
  • new Function()
  • setInterval
  • setTimeout
  • window.open

[testing] pass entire rules array instead of single rule

Currently, when running tests. We define the name of the rule we're looking for, say .innerHTML. Then we look up that rule in common/rules.js, and pass that single rule to ScanJS.scan().

This is limiting for a few reasons, but mainly, I could see use getting into a situation where we have multiple rules to ensure all dangerous uses are caught. For example, we want to catch var a = mozApps.mgmt and also var a = mozApps.mgmt.getAll().

Right now, I believe, this would require two different rules. If that is the case, then we would need two separate test files that find their specific rule and then run ScanJS.scan() twice with each rule. That violates keeping our code DRY and I could see that becoming more complicated to catch things.

Instead, if we pass the entire rules array each time, while certainly more inefficient since we go through all rules for each test file, we can have one test file that uses mozApps.mgmt in a bunch of ways and not be concerned with 'should this specific rule stop this behavior' and instead focus on 'these are bad patterns, these are good patterns, ScanJS.scan() should catch them'.

I feel pretty strongly about this, after working with tests for a while, but I definitely wanted to get @pauljt and @mozfreddyb opinions! ๐Ÿ”

mouseover or click line number in results to change editor?

I find the mouseover less intuitive than the click. The mouseover doesn't work very well once you have a big app with a lot of results, because you line number and editor are far away from each other.

Another thing I have noticed is that setCursor focuses the CodeMirror instance (for the highlighted line to be actually visible), but that means the whole web page scrolls up..which is also annoying for big pages.

Example screenshot (click to enlarge):

[testing][false positive] - variable named same as object

A lot of our rules flag variables named the same as the object as dangerous, when the shouldn't.

Currently, we have the following failing test cases:

     CustomEvent -> good.CustomEvent = "CustomEvent";
     addEventListener -> var addEventListener = "variable with name";
     addIdleObserver -> addIdleObserver = "static string";
     addIdleObserver -> something.addIdleObserver = "static string";
     escapeHTML -> var escapeHTML = "just a string";
     localStorage -> var localStorage = "asdf";
     message -> var message = "static string";
     MozActivity -> var MozActivity = "static MozActivity";
     MozChromeEvent -> var mozChromeEvent = "string mozChromeEvent";
     mozPermissionSettings -> var mozPermissionSettings = "just a string, not mozPermissionSettings";
     mozPower ->  var mozPower = "just a string, window.navigator.mozPower;";
     mozSettings -> var mozSettings = "window.navigator.mozSettings;";
     mozSms -> var mozSms = "window.navigator.mozSms;";
     somethingNotNavigator.mozWifiManager;

The following rules need be updated:

  • CustomEvent
  • addEventListener
  • addIdleObserver
  • escapeHTML
  • localStorage
  • message
  • MozActivity
  • MozChromeEvent
  • mozPermissionSettings
  • mozPower
  • mozSettings
  • mozSms
  • mozWifiManager

Different rule sets depending on target

It would be cool if we had different rule sets, depending on the target platform (nodejs, browser, ...)

I was looking into Cordova (aka PhoneGap) earlier today and realized most of the browser cases already cover this. There are some other APIs, but those highly rely on the app and which Cordova plugins are being used.

But this here lists a few items/APIs that aren't in the current rules yet:

  • requestFileSystem
  • standard HTML FileSystem API, not in Firefox as of Feb 2014 though
  • FileReader
    • standard HTML File API, already in Firefox, not sure if interesting enough
  • Media
    • for media playback
  • navigator.contacts
    • probably shims mozContacts, but its more interesting to see the functions the developers uses, not her libraries ;)
  • navigator.accelerometer

idea: support multiple file upload

It would be nice if one could upload multiple JS files at the same time.
This requires multiple codeMirror instances per file (or changing the content on the fly), some additional file navigation. Question is, would the "Output" part belong to a specific file tab or continue showing for all files?

(This surely wouldn't look nice if #4 isn't fixed though.)

Hosting ScanJS with a CSP?

We should probably make it CSP compliant, just to 'dog-food' :)

If we keep using angular.js, ngCsp seems relevant.

Otherwise, its just a matter of moving everything to externally sourced files. This is definitely super low priority but might be good to keep in mind for new features we implement.

[Rules] - Update functionality does not work

This does not seem completely reproducible.

  1. Navigate to http://people.mozilla.org/~rfletcher/scanjs/client/
  2. Click rules tab
  3. select '.innerHTML'
  4. Change rule_desc from: "Assignments to innerHTML with user input can lead to XSS." to: "Assignments to inner<img/src=a onerror=alert(0)>HTML with user input can lead to XSS."

The following error in console:

Error: current is not defined RuleListCtrl/$scope.saveRule@http://people.mozilla.org/~rfletcher/scanjs/client/js/rules.js:14 Za.prototype.functionCall/<@http://people.mozilla.org/~rfletcher/scanjs/client/js/lib/angular-1.2.13.min.js:164 Pc[c]</<.compile/</</<@http://people.mozilla.org/~rfletcher/scanjs/client/js/lib/angular-1.2.13.min.js:181 Cd/this.$get</h.prototype.$eval@http://people.mozilla.org/~rfletcher/scanjs/client/js/lib/angular-1.2.13.min.js:103 Cd/this.$get</h.prototype.$apply@http://people.mozilla.org/~rfletcher/scanjs/client/js/lib/angular-1.2.13.min.js:104 Pc[c]</<.compile/</<@http://people.mozilla.org/~rfletcher/scanjs/client/js/lib/angular-1.2.13.min.js:181 o.event.dispatch@http://people.mozilla.org/~rfletcher/scanjs/client/js/lib/jquery-2.1.0.min.js:3 o.event.add/r.handle@http://people.mozilla.org/~rfletcher/scanjs/client/js/lib/jquery-2.1.0.min.js:3


return a.apply(null,arguments)}

[Feature Request] - new layout

This issue is to track potential changes to UX layout.

Currently, we have an Input section and an Output section. A better solution might be to simply render the entire JS file in one view and then highlight lines with errors. Also, provide functionality to 'jump' to errors inside files in case file is really large. This will clean up interface and allow best use of real estate.

[testing][add rule] - crypto

create a rule for crypto.

Related to generateCRMFRequest test:

var a = crypto;
a.generate = crypto.generateCRMFRequest;
a.generate("CN=0", 0, 0, null, "console.log(1)", 384, null, "rsa-dual-use");

testing, yay or nay

(Copied over from the old repo, originally filed by @pwnetrationguru)

I'm thinking it would be good to have some light tests that we run before merging in PRs.
I don't think this is too heavy handed and I have experience writing some light testing, but I wanted to get everyone's opinion.
@mozfreddyb and @pauljt

include http server vs. using file://

We need to switch to providing a lightweight HTTP server so a user can go to http://localhost:9999/scanjs/client vs file:///Users/rfletcher/work/scanjs/scanjs/client/index.html

Right now, in Chrome, we have cross-origin loading errors. More importantly, we use relative file paths throughout the code. This is presenting problems and isn't really how we should be doing it.

For example:
scanjs/client/index.html and scanjs/tests/run-tests.html include scanjs/client/js/initTern.js. scanjs/client/js/initTern.js attempts to load the file js/lib/tern/defs/browser.json.

This means index.html is looking for file js/lib/tern/defs/browser.json inside scanjs/client while run-tests.html is looking for js/lib/tern/defs/browser.json inside of scanjs/tests/

If we had a lightweight HTTP server, initTern.js could simply do a
getJSON('browser', '/scanjs/client/js/lib/tern/defs/browser.json');

[testing] false positive relating to identifiers

Consider the following test for mozSettings rule. Specifically, pay attention to the second 'safe input' test that uses var mozSettings = "window.navigator.mozSettings;";

... Testing Safe Input
      var good = 'var a = "window.navigator.mozSettings;";';
      chai.expect(ScanJS.scan(good, [ScanJS.rules[i]], document.location.pathname)).to.be.empty;
      var good = 'var mozSettings = "window.navigator.mozSettings;";';
      chai.expect(ScanJS.scan(good, [ScanJS.rules[i]], document.location.pathname)).to.be.empty;
... Testing Unsafe Input
      var bad = 'var settings = window.navigator.mozSettings;';
      chai.expect(ScanJS.scan(bad, [ScanJS.rules[i]], document.location.pathname)).not.to.be.empty;});
...

Basically, the var mozSettings = "window.navigator.mozSettings;"; test is failing. I would think it should not since var mozSettings should be different from window.navigator.mozSettings.

I think this is the behavior with all identifier rules.

@pauljt and @mozfreddyb, what do you think? I've still yet to really dig into the guts of ScanJS.scan() so I don't know if this is possible or not.

visual ui feedback

I'd like to introduce a div tag to index.html that are styled to be well placed (i.e. top, center, overlay) to provide visual feedback for all sorts of UI interactions like "scan complete", "scan started", "file loading" etc.

This div tag shall have the alert and one of the alert-info (blue), alert-error (red) and alert-warning (yellow) classes attached to it whenever we call a function that triggers those visual clues.

Those classes should automatically be removed after a certain timeout, e.g. 1 second.
I guess it's unlikely that one would want to display more than one of those things - otherwise we could just include three tags instead of one that is repurposed all the time.

Any comments, @pwnetrationguru? Otherwise I'll take this soonish :)

[Feature Request] - 'tainted' object false positives (type inference)

The following input:

var a = 'something not bad';
document.innerHTML = a;
var b = document.location;
document.innerHTML = b;

Produces:

 Line 2:
document.innerHTML = a;
Line 4:
document.innerHTML = b; 

Since we know a is a static string, it would be nice if line 2 did not produce a warning. This might be non-trivial.

[testing][false negative] - setAttribute

Using .setAttribute should flag dangerous cases. We are missing the following examples:

var a=document.createElement("form"); a.setAttribute("action", "demo_form.asp"); document.body.appendChild(a);
var a = document.createElement("a"); a.setAttribute("href", "javascript:alert(0)"); document.body.appendChild(a);
iframe.setAttribute("mozapp", data.app);
var a = document.createElement("audio"); a.setAttribute("mozaudiochannel", data.app);

The following rules having pending tests relating to setAttribute:

  • action attribute
  • href attribute
  • mozapp
  • mozaudiochannel

This enhancement will be fixed post-1.0 release.

Prettify source prior to scanning

This would help with libraries and is mandatory for app reviews. Viewing code appears pretty useless without this.

I think this should be an option and the user should be allowed to disable.

web workers console.log causes error

We cannot use console.log inside a web worker, it causes console not defined errors. If we need to pass messages back and forth, we should use postMessage somehow.

Right now, scan.js uses console.log() pretty extensively. So we either need to add conditionals in there to only call console.log() when its being used NOT in a web worker, or we need to remove all console.logs from scan.js.

I went ahead and assigned @mozfreddyb, hope you don't mind :)

We should try to get this fixed ASAP, as master is broken.

[Feature Request] - server output to files needs to be more flexible

Right now, if you run the server side scan.js after having already run a scan you get this error:

Error:output file or dir already exists (scanresults). Supply a different name using: -o [filename]

This feature request would create file/directories with timestamped dates so that the user can run multiple tests.

[testing][false negative] - building HTML from innerHTML static strings

Relating to our action attribute rule, ScanJS misses:

var a=document.createElement("div");
a.innerHTML="<form action='demo.asp'></form>";
document.body.appendChild(a);

We do not flag this because innerHTML is assigned from a static string.

We should flag this because we are building a <form> with the action attribute.

rule description is not obvious

The description is hidden in the rule name's title attribute. It should be more obvious. We might tackle this in another issue, but I wanted this to be spelled out explicitly

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.