Skip to content

Commit

Permalink
Add rule to check if opener was disowned
Browse files Browse the repository at this point in the history
Fix #149
  • Loading branch information
alrra authored and molant committed May 2, 2017
1 parent 01b8018 commit db72999
Show file tree
Hide file tree
Showing 8 changed files with 457 additions and 2 deletions.
1 change: 1 addition & 0 deletions .sonarrc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"formatter": "json",
"rules": {
"disallowed-headers": "warning",
"disown-opener": "warning",
"lang-attribute": "warning",
"manifest-exists": "warning",
"manifest-file-extension": "warning",
Expand Down
165 changes: 165 additions & 0 deletions docs/user-guide/rules/disown-opener.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
# Require external links to disown opener (`disown-opener`)

`disown-opener` warns against not using `rel="noopener noreferrer"`
on `a` and `area` elements that have `target="_blank"` and link to
other origins.


## Why is this important?

Links that have `target="_blank"`, such as
`<a href="https://example.com" typeof="_blank">` constitute:

* [a security problem](https://mathiasbynens.github.io/rel-noopener/)

When using `typeof="_blank"` the page that was linked to gains access
to the original page's [`window.opener`](https://developer.mozilla.org/en-US/docs/Web/API/Window/opener).
This allows it to redirect the original page to whatever it wants,
like for example, a phishing page designed to look like a real page
frequently used by users, asking for login credentials (see also: [tab
nabbing](http://www.azarask.in/blog/post/a-new-type-of-phishing-attack/)).

By adding `ref="noopener"` (and `noreferrer` for older browsers)
the `window.opener` reference won't be set, and thus, the ability
for the page that was linked to from redirecting the original one
is removed.

* [performance problem](https://jakearchibald.com/2016/performance-benefits-of-rel-noopener/)

Most modern browser are multi-process. However, due to the
synchronous cross-window access the DOM allows via `window.opener`,
in most browsers pages launched via `target="_blank"` end up in the
same process as the origin page, and that can lead to pages
experiencing jank.

In Chromium based browser, using `ref="noopener"` (or
[`rel="noreferrer"`](https://blog.chromium.org/2009/12/links-that-open-in-new-processes.html)
for older version), and thus, preventing the `window.opener` reference
from being set, allows new pages to be opened in their own process.

Edge is not affected by this.

Notes:

* Not all browsers [support](http://caniuse.com/#feat=rel-noopener)
`rel="noopener"`, so in order to ensure that things work as expected
in all browsers, for the time being, this rule requires
`rel="noopener noreferrer"`.

* The reason why the rule does not check the same origin links by
default is because:

* Security isn't really a problem here.
* When it comes to performance, making same origin links open in
their own process actually works against optimizations that some
browsers do in order to keep multiple same origin tabs within
the same process (e.g. share the same event loop).

Check [`Can the rule be configured?`](#can-the-rule-be-configured)
section to see how the rule can be made to also check same origin
links.

* [`noopener` and `noreferrer` only work for `a` and `area` elements](https://html5sec.org/#143).

* In the future there may be a [CSP valueless
property](https://github.com/w3c/webappsec/issues/139) property that
will prevent the `window.opener` reference from being set.


## What does the rule check?

By default, the rule checks if `rel="noopener noreferrer"` was specified
on `a` and `area` elements that have `target="_blank"` and link to other
origins.

Let's presume the original page is `https://example1.com`.

Examples that **trigger** the rule:

```html
<a href="http://example1.com/example.html" target="_blank">example</a>
```

```html
<a href="https://en.example1.com" target="_blank">example</a>
```

```html
<a href="//example2.com" target="_blank">example</a>
```

```html
<a href="https://example2.com" target="_blank">example</a>
```

```html
<img src="example.png" width="10" height="10" usemap="#example">
<map name="example">
<area shape="rect" coords="0,0,5,5" href="http://example3.com/example.html" target="_blank">
</map>
```

Examples that **pass** the rule:

```html
<a href="/" target="_blank">example</a>
```

```html
<a href="example.html" target="_blank">example</a>
```

```html
<a href="https://example1.com/example.html" target="_blank">example</a>
```

```html
<a href="http://example1.com/example.html" target="_blank" rel="noopener noreferrer">example</a>
```

```html
<a href="https://en.example1.com/example.html" target="_blank" rel="noopener noreferrer">example</a>
```

```html
<a href="//example2.com" target="_blank" rel="noopener noreferrer">example</a>
```

```html
<a href="https://example2.com" target="_blank" rel="noopener noreferrer">example</a>
```

```html
<img src="example.png" width="10" height="10" usemap="#example">
<map name="example">
<area shape="rect" coords="0,0,5,5" href="example.html" target="_blank">
</map>
```

```html
<img src="example.png" width="10" height="10" usemap="#example">
<map name="example">
<area shape="rect" coords="0,0,5,5" href="http://example3.com/example.html" target="_blank" rel="noopener noreferrer">
</map>
```


## Can the rule be configured?

`includeSameOriginURLs` can be used to specify that same origin URLs
should also include `rel="noopener noreferrer"`.

```json
"disown-opener": [ "warning", {
"includeSameOriginURLs": true
}]
```


## Further Reading

* [The security benefits of `rel="noopener"`](https://mathiasbynens.github.io/rel-noopener/)
* [The performance benefits of `rel="noopener"`](https://jakearchibald.com/2016/performance-benefits-of-rel-noopener/)
* [Bypassing `window.opener` protection of `rel="noreferrer"`](https://html5sec.org/#143)
* [Link type `"noopener"`](https://html.spec.whatwg.org/#link-type-noopener)
* [Link type `"noreferrer"`](https://html.spec.whatwg.org/#link-type-noreferrer)
1 change: 1 addition & 0 deletions docs/user-guide/rules/index.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# List of core rules

* [`disallowed-headers`](disallowed-headers.md)
* [`disown-opener`](disown-opener.md)
* [`lang-attribute`](lang-attribute.md)
* [`manifest-exists`](manifest-exists.md)
* [`manifest-file-extension`](manifest-file-extension.md)
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
"pify": "^2.3.0",
"request": "^2.79.0",
"require-uncached": "^1.0.3",
"same-origin": "^0.1.1",
"shelljs": "^0.7.6",
"strip-bom": "^3.0.0",
"strip-json-comments": "^2.0.1",
Expand Down
121 changes: 121 additions & 0 deletions src/lib/rules/disown-opener/disown-opener.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/**
* @fileoverview Check if `rel="noopener noreferrer"` was specified
* on `a` and `area` elements that have `target="_blank"` and link to
* other origins.
*/

// ------------------------------------------------------------------------------
// Requirements
// ------------------------------------------------------------------------------

import * as url from 'url';

import * as sameOrigin from 'same-origin';

import { debug as d } from '../../utils/debug';
import { IElementFoundEvent, IRule, IRuleBuilder } from '../../types'; // eslint-disable-line no-unused-vars
import { RuleContext } from '../../rule-context'; // eslint-disable-line no-unused-vars

const debug = d(__filename);

// ------------------------------------------------------------------------------
// Public
// ------------------------------------------------------------------------------

const rule: IRuleBuilder = {
create(context: RuleContext): IRule {

let includeSameOriginURLs = false;

const loadRuleConfigs = () => {
includeSameOriginURLs = (context.ruleOptions && context.ruleOptions.includeSameOriginURLs) || false;
};

const validate = async (data: IElementFoundEvent) => {
const { element, resource } = data;

if (element.getAttribute('target') !== '_blank') {
debug('No `target="_blank"` found');

return;
}

const hrefValue = element.getAttribute('href');

if (hrefValue === null) {
debug('`href` is not specified');

return;
}

let fullURL = hrefValue;

if (!url.parse(hrefValue).protocol) {
fullURL = url.resolve(resource, hrefValue);
}

// Same origin URLs are ignored by default, but users can
// change that by setting `includeSameOriginURLs` to `true`.

if (sameOrigin(resource, fullURL) && !includeSameOriginURLs) {
debug('Same origin not included and same origin link');

return;
}

const relValues = (element.getAttribute('rel') || '').split(' ');
const missingRelValues = [];

// TODO: In the future, only recommended `noreferrer`
// if target browsers don't support `noopener`.
//
// https://github.com/MicrosoftEdge/Sonar/issues/134

['noopener', 'noreferrer'].forEach((e) => {
if (!relValues.includes(e)) {
missingRelValues.push(e);
}
});

if (missingRelValues.length > 0) {
const location = await context.findInElement(element, hrefValue);

await context.report(resource, element, `Missing link type${missingRelValues.length === 1 ? '': 's'} on \`${await element.outerHTML()}\`: ${missingRelValues.join(', ')}`, location);
}
};

loadRuleConfigs();

// `noopener` and `noreferrer` work only with the
// `a` and `area` elements:
//
// * https://html.spec.whatwg.org/#link-type-noopener
// * https://html.spec.whatwg.org/#link-type-noreferrer
// * https://html5sec.org/#143
//
// In the future there may be a CSP valueless property:
//
// * https://github.com/w3c/webappsec/issues/139

return {
'element::a': validate,
'element::area': validate
};
},
meta: {
docs: {
category: 'security',
description: 'Use `noopener` and `noreferrer` on `a` and `area` element with target="_blank"',
recommended: true
},
fixable: 'code',
schema: [{
additionalProperties: false,
properties: { includeSameOriginURLs: { type: 'boolean' } },
type: ['object', null]
}],
worksWithLocalFiles: true
}
};

export default rule;
2 changes: 1 addition & 1 deletion tests/helpers/rule-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export const testRule = (ruleId: string, ruleTests: Array<RuleTest>, options?: o
}

return reports.forEach((report, index) => {
t.is(results[index].message, report.message, `Different message`);
t.is(results[index].message, report.message.replace(/http:\/\/localhost\//g, `http://localhost:${server.port}/`), `Different message`);
if (report.position) {
t.is(results[index].column, report.position.column, `Different column`);
t.is(results[index].line, report.position.line, `Different line`);
Expand Down
1 change: 0 additions & 1 deletion tests/helpers/test-server.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/**
* @fileoverview Simple HTTP server used in sonar's tests to mimick certain scenarios.
*
*/

import * as http from 'http';
Expand Down
Loading

0 comments on commit db72999

Please sign in to comment.