JavaScript における XSS 対策

目的
----

JavaScript 製のソフトウェアに対する汎用的な XSS 対策に関して調査・実装を行う。

動機、材料
---------

12/05 に「[俺的MarkdownにおけるXSS](https://qiita.com/Azunyan1111/items/64d23396bcf2213af024)」という記事が出ていた。Example がよくまとまっていて、試しに JavaScript で作られている OSS、[crowi-plus](https://github.com/weseek/crowi-plus) に入れてみると 動くわ動くわ。


成果物
------

https://github.com/weseek/crowi-plus/pull/217/files
(master にはマージ済み、リリースは来週)


前提
----

crowi-plus は、以下のように markdown を描画、保存している。

1. ユーザーAが、フロントエンド(jQuery, React)で markdown を書く
    1. フロントエンドは marked を利用して HTML プレビューを行う
    1. sanitize オプションは true になっている
2. 「保存」ボタン押下で、フロントエンドがバックエンド(Express)に markdown の String を送信し、MongoDB に保存する
3. ユーザーBが当該ページを閲覧
    1. バックエンドが保存されている markdown の String がフロントエンドに送られる
    1. フロントエンドが marked を利用して HTML 化する

上記動作により、

```html
<script>alert('XSS');</script>
```

のような記述を markdown に埋め込んでおくと、1-1 と 3-2 で、XSS 可能となる。


やりたいこと
------------

- markdown の利用時に限れば、利便性の面から sanitize off にはしたくない  
  (将来的にそのオプションも用意すべきだが)
- markdown に限らない状況でも使えるライブラリはどれなのかを試したい
- 危険度の高い `<script>` や `<iframe>` タグはエスケープまたは削除し、害のない div, img, a タグ等はそのまま残したい


対策・方向性
-----------

- 最も安全を期するのであればフロント・バック両対応
    - フロントは編集プレビュー時とページ閲覧時に、安全な HTML に変換する
    - バックは MongoDB への保存時に安全な markdown 文書に変換してから保存する
- ただ、使うチームによっては、markdown 文書はプレーンのままいじりたくない(表示側での対応にとどめたい)という要求があるかもしれないので、最初はフロントのみで実装し、バックエンドでの処理を行うかどうか、あるいはフロント・バック両方で処理するかなどをユーザーが選べるようにする



ライブラリ選定
-------------

npm でダウンロードが多いのは以下。

- https://www.npmjs.com/package/x-xss-protection
- https://www.npmjs.com/package/xss-filters
- https://www.npmjs.com/package/xss

だが、github のスターでは完全に真逆。

- https://github.com/leizongmin/js-xss
- https://github.com/yahoo/xss-filters
- https://github.com/helmetjs/x-xss-protection

x-xss-protection はヘッダを付加するだけなので今回のような対策には使えない。

xss-filters はお手軽だが、ホワイトリスト機能がないので今回の要求を満たせない。

3つめの xss (leizongmin/js-xss) が高機能かつオプションで全ての要求を満たすことが出来る。



ミニマムスニペット
-------------

```javascript
import xss from 'xss';

(中略)

process(markdown) {

  return xss(markdown, {
    stripIgnoreTag: true,
    css: false,
    // allow all attributes
    option.onTagAttr = function(tag, name, value, isWhiteAttr) {
      return `${name}="${value}"`;
    }
  });

}
```

オプションで `whiteList` を指定しない場合は、デフォルトのホワイトリストが使われる。
コードはこちら -> https://github.com/leizongmin/js-xss/blob/master/lib/default.js

上記コードスニペットでは、strip されなかったタグ(つまりホワイトリストに入っているタグ)に関しては、全ての attribute を許容するようにしている。


フロント・バック共用クラス化
-------------------------

「まだ時代は来てない」とか「お行儀がよくない」とかいろいろあるかもしれないが、重複コードが嫌なので crowi-plus では node.js(6.x) と babel-env(last 2 versions) で共用できるクラスを作った。

```javascript:util/xss.js
class Xss {

  constructor(isAllowAllAttrs) {
    const xss = require('xss');

    // create the option object
    let option = {
      stripIgnoreTag: true,
      css: false,
    };
    if (isAllowAllAttrs) {
      // allow all attributes
      option.onTagAttr = function(tag, name, value, isWhiteAttr) {
        return `${name}="${value}"`;
      }
    }
    // create the XSS Filter instance
    this.myxss = new xss.FilterXSS(option);
  }

  process(markdown) {
    return this.myxss.process(markdown);
  }

}

module.exports = Xss;
```


### バックエンド(mongoose の hook)で使う場合

```javascript:model/schema.js
const Xss = require('../util/xss')

(中略)

schema.pre('save', function(next) {
  this.markdown = xss.process(this.markdown);
  next();
});
```


### フロントエンドで使う場合

よしなに


テストしてみよう
--------------

### iframe

```html
<iframe srcdoc="<script>alert('XSS');</script>"></iframe>
```
```html
<iframe src="data:text/html;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk7PC9zY3JpcHQ+">
```
この辺は大丈夫。

### onmousemove

```html
<b onmousemove="alert('XSS')">XSS!</b>
```

`isAllowAllAttrs` を true にしていると、残念ながらこれは通ってしまう。false にして class や style 等を使えるようにするのを諦めるか、またはもっとそれらの属性に限定して付加を許すべきかも。

### コードブロック + onmousemove

元記事の `.py "onmousemove='alert("XSS")'\\` のやつ。

crowi-plus ではこれは動いちゃう…のだが、実は別のところに原因が。



コードブロック処理のジレンマ
-------------------------

xss (leizongmin/js-xss) は、`whiteList` やその他のオプションのおかげで比較的柔軟なカスタムが可能ではあるのだが、バッククォートx3で囲われるコードブロックには当然 `<script>` タグが含まれることもあり、それまで処理されるのは免れられない。

crowi-plus ではそれを解決する[独自コード](https://github.com/weseek/crowi-plus/blob/c4bfdabc42c7c894dd9e676ad1c05424548a1647/resource/js/util/interceptor/detach-code-blocks.js#L44)を書いており、

1. コードブロック部分を置換して退避
2. 各種処理(img, a タグ変換や絵文字との置換、今回の XSS 対策など)を実行
3. 退避しておいたコードブロックを元に戻す

という操作を行っている。

が、XSS対策というコンテキストではこの機能が悪さをすることになってしまう。これはどっちを取るかという話になってしまうので、最早致し方ないか…


まとめ
------

* sanitize しなければそもそも起こらない問題 → sanitize しながらどこまで許容したいか
* 限定されたタグだけ許容したいなら、xss (leizongmin/js-xss) のデフォルト設定で十分
    * 更に細かく足し引きも可能
* attribute に関しては、全て許可にすると mouse, key 関係のトリガを操作できてしまうので問題が出る
* コードブロックは特殊な処理をしなければ xss (leizongmin/js-xss) に任せて大丈夫


crowi-plus での今後の課題
------------------------

やっぱりオプション化だよねってことで。

https://github.com/weseek/crowi-plus/issues/216