Javascriptのセキュリティ対策について

Question

実現したいこと

JavascriptでDom XSSの脆弱性につながるコードを修正したい

前提

還移式の掲示板を作成していたのですが、非同期通信で1画面で完結する掲示板に変更中です

該当のソースコード

<div id="input_area">
    <form name="input_form">
        <div>名前<input type="text" name="namae" id="namae"></div>
        <div>コメント<textarea name="message" id="message"></textarea></div>
        <div>
            <input type="radio" name="stamp" value="1" id="stamp_1"><label for="stamp_1"></label>
            <input type="radio" name="stamp" value="2" id="stamp_2"><label for="stamp_2"></label>
            <input type="radio" name="stamp" value="3" id="stamp_3"><label for="stamp_3"></label>
            <input type="radio" name="stamp" value="4" id="stamp_4"><label for="stamp_4"></label>
            <input type="radio" name="stamp" value="5" id="stamp_5"><label for="stamp_5"></label>
            <input type="radio" name="stamp" value="6" id="stamp_6"><label for="stamp_6"></label>
            <input type="radio" name="stamp" value="7" id="stamp_7"><label for="stamp_7"></label>
            <input type="radio" name="stamp" value="8" id="stamp_8"><label for="stamp_8"></label>
        </div>
        <div>
            <input type="file" class="attach" name="attach[]">
            <div class="viewer"></div>
        </div>
        <div>
            <input type="file" class="attach" name="attach[]">
            <div class="viewer"></div>
        </div>
        <div>
            <input type="file" class="attach" name="attach[]">
            <div class="viewer"></div>
        </div>
        <div>
            <button type="button" id="input_button">確認画面へ進む</button>
        </div>
    </form>
</div>
<div id="confirm_area"></div>
<div id="result_area"></div>
<script>
    const input_area = document.getElementById("input_area");
    const confirm_area = document.getElementById("confirm_area");
    const result_area = document.getElementById("result_area");
    var namae_value = "";
    var message_value = "";
    var stamp_value = "";
    function input_button_click() {
        namae_value = "";
        message_value = "";
        stamp_value = "";
        //サーバーにデータを送信する際に使用するオブジェクトを生成
        const formData = new FormData(input_form);
        //オブジェクト内の既存のキーに新しい値を追加
        formData.append("action", "bbs_quest_input");
        const opt = {
            method: "post",
            body: formData
        }
        //非同期通信
        fetch("<?php echo home_url('wp-admin/admin-ajax.php'); ?>", opt)
            .then(response => {
                return response.json();
            })
            .then(json => {
                if (json.error != "") {
                    alert(json.error);
                    return;
                }
                namae_value = document.getElementById("namae").value;
                message_value = document.getElementById("message").value;
                const stamps = document.getElementsByName('stamp');
                for (var stamp of stamps) {
                    //checkedプロパティは、対象の要素がcheckedを持っていればtrueを、持っていなければfalseを返す
                    if (stamp.checked) {
                        stamp_value = stamp.value;
                        break;
                    }
                }
                confirm_area.innerHTML = "";
                var div;
                var child;
                child = document.createElement("h2");
                child.innerHTML = "確認画面";
                confirm_area.appendChild(child);
                div = document.createElement("div");
                child = document.createElement("p");
                child.innerHTML = "名前：" + namae_value;
                div.appendChild(child);
                confirm_area.appendChild(div);
                div = document.createElement("div");
                child = document.createElement("p");
                child.innerHTML = "コメント：" + message_value;
                div.appendChild(child);
                confirm_area.appendChild(div);
                div = document.createElement("div");
                child = document.createElement("input");
                child.type = "radio";
                child.name = "stamp";
                child.id = "confirm_stamp";
                child.value = stamp_value;
                child.checked = true;
                div.appendChild(child);
                child = document.createElement("label");
                child.htmlFor = "confirm_stamp";
                div.appendChild(child);
                confirm_area.appendChild(div);
                div = document.createElement("div");
                for (const i in blobType) {
                    if (blobType[i] != "") {
                        child = null;
                        if (blobType[i] == "img") {
                            child = document.createElement("img");
                        } else if (blobType[i] == "video") {
                            child = document.createElement("video");
                            child.setAttribute("controls", null);
                        } else if (blobType[i] == "iframe") {
                            child = document.createElement("iframe");
                        }
                        if (child !== null) {
                            child.style.maxHeight = "200px";
                            child.style.maxWidth = "200px";
                            child.src = blobUrl[i];
                            div.appendChild(child);
                        }
                    }
                }
                confirm_area.appendChild(div);
                div = document.createElement("div");
                child = document.createElement("button");
                child.type = "button";
                child.innerText = "入力画面へ戻る";
                child.addEventListener("click", () => {
                    input_area.style.display = "block";
                    confirm_area.innerHTML = "";
                    confirm_area.style.display = "none";
                });
                div.appendChild(child);
                child = document.createElement("button");
                child.type = "button";
                child.innerText = "結果画面へ進む";
                child.addEventListener("click", confirm_button_click);
                div.appendChild(child);
                confirm_area.appendChild(div);
                input_area.style.display = "none";
                confirm_area.style.display = "block";
            })
            .catch(error => {});
    }

    function confirm_button_click() {
        const formData = new FormData();
        formData.append("action", "bbs_quest_confirm");
        const opt = {
            method: "post",
            body: formData
        }
        fetch("<?php echo home_url('wp-admin/admin-ajax.php'); ?>", opt)
            .then(response => {
                return response.json();
            })
            .then(json => {
                if (json.error != "") {
                    alert(json.error);
                    return;
                }
                result_area.innerHTML = "";
                var div;
                var child;
                child = document.createElement("h2");
                child.innerHTML = "結果画面";
                result_area.appendChild(child);
                div = document.createElement("div");
                child = document.createElement("p");
                child.innerHTML = "名前：" + namae_value;
                div.appendChild(child);
                result_area.appendChild(div);
                div = document.createElement("div");
                child = document.createElement("p");
                child.innerHTML = "コメント：" + message_value;
                div.appendChild(child);
                result_area.appendChild(div);
                div = document.createElement("div");
                child = document.createElement("input");
                child.type = "radio";
                child.name = "stamp";
                child.id = "result_stamp";
                child.value = stamp_value;
                child.checked = true;
                div.appendChild(child);
                child = document.createElement("label");
                child.htmlFor = "result_stamp";
                div.appendChild(child);
                result_area.appendChild(div);
                div = document.createElement("div");
                for (const i in blobType) {
                    if (blobType[i] != "") {
                        child = null;
                        if (blobType[i] == "img") {
                            child = document.createElement("img");
                        } else if (blobType[i] == "video") {
                            child = document.createElement("video");
                            child.setAttribute("controls", null);
                        } else if (blobType[i] == "iframe") {
                            child = document.createElement("iframe");
                        }
                        if (child !== null) {
                            child.style.maxHeight = "200px";
                            child.style.maxWidth = "200px";
                            child.src = blobUrl[i];
                            div.appendChild(child);
                        }
                    }
                }
                result_area.appendChild(div);
                confirm_area.style.display = "none";
            })
            .catch(error => {});
    }
</script>

アドバイス頂きたいこと

セキュリティについて前回もアドバイス頂いたのですが、HTMLElement.innerHTML は変数が保証されていない場合 DOM-based XSS の原因につながるようで、どう修正していくべきか悩んでおります。
Document.createTextNode を代用する場合どのように修正すれば良いのでしょうか？

参考サイト

※DOM-based XSSについて
https://gihyo.jp/dev/serial/01/javascript-security/0006

※クロスサイトスクリプティング対策
https://atmarkit.itmedia.co.jp/ait/articles/1312/17/news010_2.html
https://atmarkit.itmedia.co.jp/ait/articles/1312/17/news010_2.html

Comments

[think49]

@homepage-site さん
グローバルコード上に const, var が混在していますが、var を使う必然性があるのでしょうか。
グローバル変数にする意図があるなら var を使うべきですが、そうでないなら let, const のどちらかにするのがベターだと思います。
(let, constで宣言した変数はグローバル変数ではありません)

[homepage-site]

アドバイスありがとうございます、特に意図はないため const に修正いたします。

Answer 1

テキストノード生成/挿入

Document.createTextNode を代用する場合どのように修正すれば良いのでしょうか？

createTextNode() で生成したテキストノードは appendChild() で挿入可能です。
用途に応じて、下記APIを使い分ければよいと思います。

用途	方法
テキストノード生成	createTextNode()
テキストノード値を書き換え	テキストノードの data プロパティにテキスト代入
要素ノードの全ての子ノードを削除してテキストノード挿入	要素ノードの textContent プロパティにテキスト代入

Comments

[homepage-site]

回答ありがとうございます、質問の内容が伝わりずらく申し訳ありません。
Element: innerHTML で空の要素を追加したいのですが、createTextNode() で可能なのでしょうか？
以前いただいたアドバイスで Document.createTextNode を使用するかエスケープ処理を行う方法の2つがあると教えて頂いたのですが、値がないものもあるためどうすべきか悩んでおります。

[think49]

@homepage-site さん
createElementで空要素ノードを生成し、appendChildで挿入して下さい。
createTextNodeはテキストノートを生成するAPIです。

[homepage-site]

回答ありがとうございます、空の要素を追加したい場合はどうすればいいのでしょうか？

※考えたコード（要素が空の場合）
confirm_area.innerHTML = "";
↓
confirm_area.textContent = '';

（ノードの書き換えを行う場合）
child = document.createElement("h2");
child.innerHTML = "確認画面";
↓
child = document.createElement("h2");
child.append('確認画面');

[think49]

@homepage-site さん
あなたが書いたコードは「空の要素を追加」ではありません。
DOMの世界では要素とは「要素ノード」のことです。
そのコードは「空の要素ノードを追加」していますか？
正しい用語を使わなければ、相手に伝わりませんのでご注意ください。

期待する動作が「要素の子ノードを全て削除」であるなら、あなたの書いたコードでも問題ありません。
正攻法で書くなら、element.childNodes を繰り返し処理で remove もしくは removeChild することになります。
XSSを気にするなら innerHTML の使用を可能な限り控えた方がよく、子ノードにテキストノードのみが存在するなら textContent を採用、そうでないなら、remove, removeChild を採用、と私なら考えます。
どれでも挙動は同じですが、コードを読む人に意図が伝わりやすいことが大切だと思います。

[homepage-site]

A.回答ありがとうございます、失礼いたしました。要素を空にするというほうが正しい表現でした。
子ノードにテキストノードのみが存在するということについてお聞きしたいのですが、作成中の要素はユーザーが書き込んだ名前、コメント、選択したスタンプの数字、画像、動画の
blobType になります。
この場合 remove, removeChild が適切だと思うですが如何でしょうか？

テキストノード生成する場合は createTextNode()
要素を空にする場合は textContent または、remove, removeChild を採用するという解釈で問題ないでしょうか？

[think49]

@homepage-site さん
> 作成中の要素はユーザーが書き込んだ名前、コメント、選択したスタンプの数字、画像、動画のblobType になります。
> この場合 remove, removeChild が適切だと思うですが如何でしょうか？

element.childNodes の中身によります。

(A) element.childNodes がテキストノードのみ
element.textContent = '';

(B) element.childNodes にテキストノード以外(要素ノード等)がある
element.childNodes を remove(), removeChild()

子ノードが単一なら、element.removeChild(element.firstChild) でも良いでしょう。

> テキストノード生成する場合は createTextNode()
> 要素を空にする場合は textContent または、remove, removeChild を採用するという解釈で問題ないでしょうか？

問題ないと思います。

[homepage-site]

アドバイスありがとうございます、HTMLElement.innerHTML の状態でサンプルサイトから nodeType を検索したところ、nodeType: 1 と表示されており、このノードの種類を識別したところ要素ノードの Element に分類されているようです。
コードを書き換える前ですがこちらの情報を頼りにしても良いのでしょうか？

※nodeType を調べる方法
https://qiita.com/andota05/items/a2292d2b7780ed5faa31

※nodeType プロパティ
https://developer.mozilla.org/ja/docs/Web/API/Node/nodeType

※ノードクラスについて
https://ja.javascript.info/basic-dom-node-properties

※サンプルサイト
http://oksample.starfree.jp/%E8%B3%AA%E5%95%8F%E6%8E%B2%E7%A4%BA%E6%9D%BF/

上記とは別の質問になるのですが、子ノードが単一の場合というのはどのような場合になるのでしょうか？

[think49]

@homepage-site さん
> コードを書き換える前ですがこちらの情報を頼りにしても良いのでしょうか？

基本的に個人サイトの情報を頼りにするのは危険だと思います。
調査のとっかかり(キーワードを知る等)としては有ですが、他に信頼できるサイトで裏どりをする必要があります。
挙げられたサイトの中ではMDNはFirefox開発している団体のサイトなので9割方は信頼してよい(細かな部分で間違っている事はあります。MDN日本語版の情報が古く、MDN英語版に最新情報があることもあります。)ですが、他は裏どりが必要だと思います。
MDNだけでも大体の情報はそろいますので、個人サイトでキーワードを抑えた後にMDNで調べなおすのが良いと思います。
信頼度としては「個人サイト >>> MDN >> 仕様書」が目安となり、仕様書が最も信頼できる情報源となります。
teratailも私の意見も鵜呑みにせず、信頼できる情報源で裏どりしてください。

MDNからリンクされている仕様書は英語版ですが、仕様書のタイトルで検索すると日本語訳サイトが見つかることが多いです。
https://triple-underscore.github.io/DOM4-ja.html

※少し情報が古いですが、私が過去に書いた記事を紹介しておきます。
https://gist.github.com/think49/689d7d1e5c5fd0c5ca266e4c66b0b35e#%E5%8F%82%E8%80%83%E8%B3%87%E6%96%99
https://gist.github.com/think49/2f02c7371dded1ce5aab5da3fac122cf

[homepage-site]

アドバイスありがとうございます。サイトですべてを解決しようとしていたので MDN で裏どりを行うようにいたします。
think49さんが書いておられる記事も読ませていただきます。

Answer 2

innerHTMLの代替はtextContentでいいと思います。

それはそれとして、読みにくくて現実的ではないコードなのでテンプレートリテラルやtemplate要素を使うなどして書き直すことをおすすめします。

Comments

[think49]

@Lhankor_Mhy さん
> テンプレートリテラルやtemplate要素を使うなどして書き直すことをおすすめします。

テンプレートリテラルはXSS対策の観点で見ると、不向きな側面があると思います。
- 普通に書くと「属性値」「テキストノード値」をエスケープしないので、XSS対策の為に特別なエスケープ処理が必要
- innerHTML か insertAdjacentHTML を使用する必要があり、XSS対策できているかチェックするのに時間がかかる
https://jsfiddle.net/yxboLvsk/2/

一方、template要素はinnerHTML,insertAdjacentHTMLに頼る必要がなく、シンプルかつXSS対策のチェックが容易ですね。

[Lhankor_Mhy]

説明が足りなかったですが、テンプレートリテラルでスケルトンのDOMを作って、textContent などでテキストを挿入した方が見通しがいいだろう、という考えでした。

まあ、そもそもこの用途なら、確認部分を普通にHTMLで記述して非表示にしておくだけで十分ですしね……

[think49]

@Lhankor_Mhy さん
なるほど。https://jsfiddle.net/yxboLvsk/2/ のgood2と同じ使い方ですね。
このパターンを書いて少し迷ったのですが、「HTML文字列からDOMノードに変換する処理」はどのようにされているでしょうか。
私は下記2パターンを思いつきましたが、いずれも渡されたHTML文字列を配置する場所によってはHTML文法違反の可能性があり、期待外のDOMノードが生成される可能性があると考えています。

(A) createElement でダミー要素生成→innerHTML or insertAdjacentHTML→childNodesをdfに挿入
(B) DOMParserでHTMLDocument生成→importNode

[Lhankor_Mhy]

good2の方を書く場合は(A)で書いてました。DocumentFragment は template 要素を使うためのインターフェース、みたいなイメージがあります。

しかし、たしかに、テーブルなどを作る時には容易に文法違反しそうですね。
あまり考えてなかったですが、これ、template要素からクローンしても同じ問題は起きますね……
回避方法としては、挿入先要素を DocumentFragment の中にクローンして、挿入要素を挿入し、parentElement を調査するとか……？　漏れもありそうだし、メモリにも速度にもよろしくなさそう……

[Lhankor_Mhy]

ああ、そういえばよく考えると、(A) だと、DocumentFragment に渡す前に文法違反する可能性もありましたね。
そうすると、(B)の方が汎用的なのかな……？

[Lhankor_Mhy]

いや、div じゃなくて、template にすればいいだけの話でしたね。失礼しました。

[think49]

@Lhankor_Mhy さん
good1はエスケープ処理をテキストノードに限定しているため、文法違反のリスクはないと考えています。

一方、good2は (A) を採用し、div要素にinnerHTMLしています
div要素のコンテンツモデルはフローコンテンツ、パルパブルコンテンツであり、無制限ではないので文法違反のリスクがあります。
https://momdo.github.io/html/grouping-content.html#the-div-element

(B)の場合もHTML文字列を挿入する場所が課題になり、body要素配下に置くことを考えていましたが、body要素のコンテンツモデルは「フローコンテンツ」なので、無制限ではなさそうです。
https://momdo.github.io/html/sections.html#the-body-element

(A)も(B)も親要素のコンテンツモデルに縛られる点では問題の本質は同じと考えています。

> good2の方を書く場合は(A)で書いてました。DocumentFragment は template 要素を使うためのインターフェース、みたいなイメージがあります。

DocumentFragmentは複数ノードを生成して挿入する為のAPIというイメージでした。
template要素の場合、コンテンツモデルがNothingなので、ほぼ制約はないと思っています。
https://momdo.github.io/html/scripting.html#the-template-element
気を付けるべきはtemplate要素からimportしたノードの挿入先ですが、そこはコード上の問題なので何とかなるかなと。

[think49]

> いや、div じゃなくて、template にすればいいだけの話でしたね。失礼しました。

確かに、(A)も(B)もtemplate要素の配下に置けば解決できそうですね。
試してみます。

[think49]

template要素を使って実装できました。
https://jsfiddle.net/Lpz47835/

(A)がgood2、(B)がgood3となります。
この解決法は良いですね。

[Lhankor_Mhy]

あー、なるほど。
たしかに、私の方で問題の整理ができていませんでしたね。ご提示のgood2もgood3も汎用的で使えそうです。ありがとうございます。

[homepage-site]

回答ありがとうございます、要素内を空にしたい場合は textContent ノードの置き換えを行い場合は createTextNode を使うというのが最適でしょうか？

template要素について初めて知ったのですが、要素が複数ある場合コードの整理が難しくなることはないのでしょうか？

[Lhankor_Mhy]

> createTextNode を使うというのが最適でしょうか？

createTextNode はテキストノードを作るものですので、不適です。

> 要素が複数ある場合コードの整理が難しくなることはないのでしょうか？

想定されているケースが思い浮かびませんでした。

[homepage-site]

回答ありがとうございます、createTextNode は不適切だという事は理解できたのですが、空の要素を追加したい場合はどうすればいいのでしょうか？

※考えたコード（要素が空の場合）
confirm_area.innerHTML = "";
↓
confirm_area.textContent = '';

（ノードの書き換えを行う場合）
child = document.createElement("h2");
child.innerHTML = "確認画面";
↓
child = document.createElement("h2");
child.append('確認画面');

[Lhankor_Mhy]

あ、ノードの置き換えというのは、テキスト挿入時のことでテキストノードに限る話ですか？　であれば大丈夫です。