JavascriptでXSSの脆弱性対策を行いたい

質問

JavascriptのXSS対策としてinnerHTMLでHTMLのエスケープ処理コードを書いてみたのですが、正しいのか分かりませんアドバイスお願い致します。

※名前HTML
<div class="msg_partial">あと<strong>50</strong>文字</div>

※コメントHTML
<div class="msg_partial">あと<strong>500</strong>文字</div>

※該当コード

<h2>名前<span class="required">※必須</span></h2>
<input class="length_input" data-maxlength="<?php echo MAX_LENGTH::NAME; ?>" type="text" name="namae" id="name" placeholder="未入力の場合は、匿名で表示されます" value="<?php echo $namae; ?>">
<div class="msg_partial"></div>

<h2>コメント<span class="required">※必須</span></h2>
<textarea class="length_input" data-maxlength="<?php echo MAX_LENGTH::MESSAGE; ?>" name="message" id="message" placeholder="荒らし行為や誹謗中傷や著作権の侵害はご遠慮ください"><?php echo $message; ?></textarea>
<div class="msg_partial"></div>

※エスケープ処理を追加
function lengthCheck() {
const left = this.dataset.maxlength - this.value.length;
if (left >= 0) {
const a = document.querySelectorAll('msg_partial');
return a.innerHTML;
this.nextElementSibling.innerHTML = '<strong>' + escapeHTML(left) + '</strong>文字';
this.dataset.submit_disabled = this.value.length === 0;
} else {
const b = document.querySelectorAll('msg_partial');
return b.innerHTML;
this.nextElementSibling.innerHTML = '<strong>' + escapeHTML(-left) + '</strong>文字超過しています';
this.dataset.submit_disabled = true;
}

回答No.27

AsarKingChang · ベストアンサー

<div class="submit_button_validation submit_button_validation1">残り<span></span>文字入力できます。</div>
<div class="submit_button_validation submit_button_validation2"><span></span>文字超過しています。</div>

このまま代入しちゃえばいいんですよ！。
submit_button_validation1だけを使って
innerHTML="残り<span>"+v+"</span>文字入力できます。";
の時と、
innerHTML="span>"+v+"</span>文字超過しています。";
の時を、ifで制御すればOKですよ。
vを「escapeHTML(文字列)」するかは、自由ですが。
手っ取り早いのは(v | 0)だけでもいいかも。
＃なぜなら、vは数字を前提としてるので、それ以外を
考慮する必要はないですから。
「escapeHTML(文字列)」これを使うのは、
外部から来た情報に対してです。
だから、ユーザーが入力したものは絶対に表示しない！
って言ったのです！！

もう、そろそろ、完成じゃないかな？

補足

アドバイスありがとうございます、ifで入力文字数が制限を超えているかいないかでHTML表示を分岐するように考えてみたのですが、
表示するHTML部分は空白でも問題ないのでしょうか？
HTML表示で文字が入力されるまで空の状態にしておいて、入力後に表示するということで window.onload = function() {} のコードを参考にいたしました。window.onload = function() {} は動かない原因になりうるため
window.addEventListener('DOMContentLoaded', function() {} を代用するように変更しております。
画像のアップロード機能が名前、メッセージの前になるので、参考サイトのどちらのコードを使えばよいのか分からず教えて頂けると助かります。

回答No.26でデバッガーでステップ実行をするようにアドバイス頂いたと思うのですが、今回はボタンの活性または非活性にする機能と文字数の表示分岐部分は分けて考えております。
(v | 0)というのがどのような意味があるのか分からないので教えて頂きたいです。

※参考サイト
https://took.jp/window-onload/

<div class="title-partial parts"> <!-- title-partial + parts -->
<h2>名前(name)<span class="required">※必須</span></h2>
<div class=parts>
<input class=submit_button type="text" type="text" name="name1" id="name" data-length=32 placeholder="未入力の場合は、匿名で表示されます" value="<?php echo $namae; ?>">
<div class="submit_button_validation submit_button_validation"></div>
</div>

<div class="body-partial parts"><!-- body-partial + parts -->
<h2>コメント(comment)<span class="required">※必須</span></h2>
<div class=parts>
<input class=submit_button type="text" name="name2" id="message" data-length=40 placeholder="荒らし行為や誹謗中傷や著作権の侵害はご遠慮ください"><?php echo $message; ?>
<div class="submit_button_validation submit_button_validation"></div>
</div>

<script>
function validation_submit(f) {
const submit = document.getElementById("submit_button");
/* 判定は逆なので、逆に渡す */
/*JavaScriptの要素を活性または非活性にする */
submit.disabled = f?false:true;
};

function validation_text(parts) {
/* このpartsグループの、input=textを抽出 */
/* HTML要素を取得 */
let text=parts.getElementsByClassName('submit_button')[0];
/* バリデーション警告パーツを抽出 */
let validation=parts.getElementsByClassName('submit_button_validation')[0];

validation.style.display = 'none';

/* 例えばのチェック */
window.addEventListener('DOMContentLoaded', function() {
let wao = document.getElementsByClassName('submit_button_validation');
const left = text.dataset.length-text.value.length;
if (left >= 0) {
/* ひとまずclassは複数配置できる形式なので、見つかった最初の1個目にアタッチ */
wao[0].innerHTML="残り<span>"+v+"</span>文字入力できます。";
text.value.length === 0;
} else {
/* そのまま、2個目にアタッチ */
wao[1].innerHTML="<span>"+v+"</span>文字超過しています。";
}
})
};

/* バリデーション条件判断部分 */
function validation() {
let parts = document.getElementsByClassName('parts');
let submit=true;
for (let i=0;i<parts.length;i++) {
if (validation_text(parts[i])!=true) {
submit=false;
}
}
validation_submit(submit);
};

/* DOM構築が終わってから呼び出される初期化関数 */
function init() {
// let text = document.getElementById('submit_button');
// text.oninput = e_text;

/* ↑これを、idじゃなくてclass対応に変更↓ */

/* class=parts内の class=submit_buttonに対して設定 */
let parts = document.getElementsByClassName('parts');
for (let i=0;i<parts.length;i++) {
parts[i].getElementsByClassName('submit_button')[0].oninput = validation;
}
validation();
};

window.onload = init;
</script>

※回答No.20でv1,v2,v3をまとめたい際に教えて頂いたコード
<script>
window.onload = function() {
let wao = document.getElementsByClassName('wao');
/* ひとまずclassは複数配置できる形式なので、見つかった最初の1個目にアタッチ */
wao[0].innerHTML="えい<span>や～</span>た～";
/* そのまま、2個目にアタッチ */
wao[1].innerHTML="はらほろひれはれ～";
}
</script>

回答No.6

AsarKingChang

>innerHTMLを使わずに.innerTextで実装することも可能なんですね勉強になりました。
タグがあるかないかだけですよ。

>ユニットをdisplay:none;で隠すと何故対策になるのでしょうか？HTMLが書き換わるものは基本的に対策するべきなのか教えていただきたいです。
最初に言った通り、JavaScriptはクライアントサイドの言語です。
何をどうやろうと、対策にはなりません。
単なるバリデーション効果を生むだけです。

>1,文字が超過している場合も表示させたいのですが、HTMLに追加するだけで可能でしょうか？
今、超過テキストを出してますよ。仮で32文字にしてます。
ここを好きに書き換えればOKです。

>2,名前を50文字でコメントを500文字で制限したい場合にHTMLクラス名を同じもので実装することは可能でしょうか？

基本可能です。
ただし、バリデーションルールが不明になります。
1つ目が文字で、2つ目が数字だけなど、
条件を加えたい場合に、どうするか？って問題、
多くはdata-XXX=YYYとやって、タグに
パラメータを埋め込むことで実現させます。

それをループで回すという具合です。
IDだと、ユニーク（全体で一つ）というルールですが、
CLASSは、何回でも使えるという特性を使えばOK!

補足

Q.innerHTMLを使わずに.innerTextで実装することも可能なんですね勉強になりました。
タグがあるかないかだけですよ。
最初に言った通り、JavaScriptはクライアントサイドの言語です。
単なるバリデーション効果を生むだけです。

A.回答ありがとうございます、.innerTextはHTMLのタグなどが含まれていた場合にエスケープする機能という事ですね。
JavaScriptはあくまでバリデーションと覚えておきます、もう一つ別の箇所でHTMLの表示非表示をCSSで切り替えている部分があるのですが、innerHTMLを使ってない場合もバリデーションはした方が良いでしょうか…？

Q.文字が超過している場合も表示させたいのですが、HTMLに追加するだけで可能でしょうか？
今、超過テキストを出してますよ。仮で32文字にしてます。
ここを好きに書き換えればOKです。

A.回答ありがとうございます、失礼いたしました。書き換えてみます。

ただし、バリデーションルールが不明になります。
1つ目が文字で、2つ目が数字だけなど、
多くはdata-XXX=YYYとやって、タグに
パラメータを埋め込むことで実現させます。

それをループで回すという具合です。
IDだと、ユニーク（全体で一つ）というルールですが、
CLASSは、何回でも使えるという特性を使えばOK!

A.アドバイスありがとうございます、ルールはおなじで文字数制限だけ変更したいと考えております、以前のコードに合わせてみたのですがループの書き方が分からず…教えて頂いたコードを使わせていただきます。
CLASSは、何回でも使える利点があるのですね、IDを使われてるコードが多い気がするのですが、そちらにもメリットはあるのでしょうか？

<body>
<div class="title-partial">
<input id=uso_input data-maxlength="<?php echo MAX_LENGTH::NAME; ?>" type="text" id="name" name="name"><!-- 50文字 -->
<br>
<div class="uso_input_validation uso_input_validation1">入力してください。</div>
<div class="uso_input_validation uso_input_validation2">あと<span>わあ</span>文字入力できます。</div>
<div class="uso_input_validation uso_input_validation3"><span>わあ</span>文字超過しています。</div>
　　　　 </div>

<div class="title-partial">
<input id=uso_input data-maxlength="<?php echo MAX_LENGTH::MESSAGE; ?>" type="text" id="message" name="name"><!-- 500文字 -->
<br>
<div class="uso_input_validation uso_input_validation1">入力してください。</div>
<div class="uso_input_validation uso_input_validation2">あと<span>わあ</span>文字入力できます。</div>
<div class="uso_input_validation uso_input_validation3"><span>わあ</span>文字超過しています。</div>
　　　　 </div>

<input id=uso_submit type="submit"><br>
条件にあわせてSubmitの使用権限を操作してます。
</body>

class MAX_LENGTH
{
public const NAME = 50;
public const MESSAGE = 500;
}

回答No.5

AsarKingChang

<html>
<head>
<script>
/* バリデーション条件判断部分 */
function validation(length) {
const v1=document.querySelector(".uso_input_validation1");
const v2=document.querySelector(".uso_input_validation2");
const v3=document.querySelector(".uso_input_validation3");
const submit = document.getElementById("uso_submit");

v1.style.display =
v2.style.display =
v3.style.display = 'none';
submit.disabled = true;

/* 例えばのチェック */

/* 最小チェック */
if (length==0) {
v1.style.display = 'block';
return;
}
/* 最大チェック */
if (length>=32) {
v3.style.display = 'block';
return;
}

/* それ以外 */
v2.getElementsByTagName('span')[0].innerText=32-length;
v2.style.display = 'block';
submit.disabled = false;
};

/* input=textへの変更があれば呼び出されるリスナ(エンドポイント) */
function e_text(e) {
validation(this.value.length);
};

/* DOM構築が終わってから呼び出される初期化関数 */
function init() {
let text = document.getElementById('uso_input');
text.oninput = e_text;
validation(0);
};

window.onload = init;
</script>

<style>

/* バリデーション警告文章は「赤」に設定 */
.uso_input_validation {
display:none;
color:red;
}

/* これで、.uso_input_validation内で使ったspanタグの中は常に「青」になる */
.uso_input_validation span {
color:blue;
}

</style>

</head>

<body>
<div>嘘フォーム</div>
<input id=uso_input type="text" id="name" name="name">/32文字まで
<br>
<div class="uso_input_validation uso_input_validation1">入力してください。</div>
<div class="uso_input_validation uso_input_validation2">あと<span>わあ</span>文字入力できます。</div>
<div class="uso_input_validation uso_input_validation3">長すぎます。</div>
<input id=uso_submit type="submit"><br>
条件にあわせてSubmitの使用権限を操作してます。
</body>

</html>

ものすご～く、投げやりなサンプルでごめんね～＾＾
暇つぶしの最中のさらに暇な時間？？に
作ったので、グダグダだけど＾＾

補足

A.修正ありがとうございます、分からない点があるのでアドバイスお願い致します。innerHTMLを使わずに.innerTextで実装することも可能なんですね勉強になりました。
DOM構築が終わってから呼び出される初期化関数というところが気になったのですが、ユニットをdisplay:none;で隠すと何故対策になるのでしょうか？HTMLが書き換わるものは基本的に対策するべきなのか教えていただきたいです。

1,文字が超過している場合も表示させたいのですが、HTMLに追加するだけで可能でしょうか？
2,名前を50文字でコメントを500文字で制限したい場合にHTMLクラス名を同じもので実装することは可能でしょうか？

<body>
<div>嘘フォーム</div>
<input id=uso_input type="text" id="name" name="name">/32文字まで
<br>
<div class="uso_input_validation uso_input_validation1">入力してください。</div>
<div class="uso_input_validation uso_input_validation2">あと<span>わあ</span>文字入力できます。</div>

<div class="uso_input_validation uso_input_validation3"><span>わあ</span>文字超過しています。</div>
<input id=uso_submit type="submit"><br>
条件にあわせてSubmitの使用権限を操作してます。
</body>

回答No.4

AsarKingChang

ちょい待ってね！

>A.お忙しい中アドバイスありがとうございます、innerで書き換えたら、block;がよくわからないのですが、どのように書くのでしょうか…
>元のクラスとは別にクラスを生成して、元のユニット？HTMLをdisplay:none;で削除するようなイメージでしょうか？

この部分サンプル作りますよ。

回答No.3

AsarKingChang

ちょっと時間がないのでざっと見になりますが、
function escapeHTML(値) {
var a = document.createElement('span');
a.textContent = 値;
return a.innerHTML;
}
要素.innerHTML = '<strong>' + escapeHTML(文字列) + '</strong>';
要素内なので、
createElementを要素に対してaddElementの方が、
綺麗だと思う事と。
表示するとわかっているなら、最初からユニットは作っておき、
デフォルトのCSSでdisplay:none;スタート。
それを、innerで書き換えたら、block;
の方がいいと思いますよ。
エレメント事作る＝そこでDOM要素が発生してしまう。
ので、ミスったら、無限DOMになるので、
そっちの方が、怖い結果を生むと思いますので。

あと、要件定義をまとめてくれたら、
私の方でも、考えてみたいですが、お任せします～
（質問自体が貴方のノルマになるべきではないと考えているので）

お礼

補足に書き忘れておりました申し訳ありません。HTML部分を少し変更しております。

※削除コード
class MAX_LENGTH
{
public const NAME = 50;
public const MESSAGE = 500;
}

※HTML変更コード
<h2>名前<span class="required">※必須</span></h2>
<input class="length_input" data-maxlength="<?php echo $MAX_LENGTH_NAME; ?>" type="text" name="namae" id="name" placeholder="未入力の場合は、匿名で表示されます" value="<?php echo $namae; ?>">
<div class="msg_partial"></div>

<h2>コメント<span class="required">※必須</span></h2>
<textarea class="length_input" data-maxlength="<?php echo $MAX_LENGTH_MESSAGE; ?>" name="message" id="message" placeholder="荒らし行為や誹謗中傷や著作権の侵害はご遠慮ください"><?php echo $message; ?></textarea>
<div class="msg_partial"></div>

補足

※修正コード
function escapeHTML($MAX_LENGTH_NAME,$MAX_LENGTH_MESSAGE) {
/* div class="msg_partial"を表示させたい為 */
const a = document.createElement('div');
/* idではなくclassを設定 */
a.className = 'msg_partial';
/* createElementを要素に対してaddElement */
a.addElement = ($MAX_LENGTH_NAME,$MAX_LENGTH_MESSAGE);
return a.innerHTML;
}

function lengthCheck() {
const left = this.dataset.maxlength - this.value.length;
if (left >= 0) {
const a = document.querySelectorAll('msg_partial');
return a.innerHTML;
this.nextElementSibling.innerHTML = '<strong>' + escapeHTML(left) + '</strong>文字';
this.dataset.submit_disabled = this.value.length === 0;
} else {
const b = document.querySelectorAll('msg_partial');
return b.innerHTML;
this.nextElementSibling.innerHTML = '<strong>' + escapeHTML(-left) + '</strong>文字超過しています';
this.dataset.submit_disabled = true;
}

<style>
.msg_partial {
display:none; /* デフォルト */
text-align: right;
color: #333;
margin: 0 15px 20px 0;
}
.msg_partial strong {
color: #e52d77;
}
.hideItems {
display: none;
}
</style>

A.お忙しい中アドバイスありがとうございます、innerで書き換えたら、block;がよくわからないのですが、どのように書くのでしょうか…
元のクラスとは別にクラスを生成して、元のユニット？HTMLをdisplay:none;で削除するようなイメージでしょうか？

数字のカウントだけcolor: #e52d77;で表示させたいと考えております。

※名前HTML
<div class="msg_partial">あと<strong>50</strong>文字</div>

※コメントHTML
<div class="msg_partial">あと<strong>500</strong>文字</div>

回答No.2

AsarKingChang

this.dataset.submit_disabled = this.value.length === 0;
これは素晴らしい＾＾
ただ、このvalueはシステムが作り出すので
型のチェックなしで==0でもよかったかもね。

lengthCheckこいつがどこで呼ばれてるのかがわからないが。
多くの場合、入力時チェックとSubmitチェックがありますが。
両方ってケースは多いですよ。
入力チェックでの未入力チェック
Submitでの全必要項目が入ったかのチェックでの2回とやろうとすると、
function lengthCheck() {
const left = this.dataset.maxlength - this.value.length;
　　　　　　　　　　　　　　　　　↑
このthisがどこを指してるのか、わかりにくくなるので、
nameフィールドでの取り出しにした方が、
メンテナンス性は上がると思います。
5個の入力枠があって全部同じならいいですが。
大抵は数字だけ欲しい、文字だけ欲しいなど
バリデーションのやり方も違うでしょうから。

最後に、本気ならJSでチェックしても無駄かと。
JSってデバッガーで好きに書き換えられるので、
どんなパラメタも勝手に追加削除できちゃうものなので。
PHPなら、サーバー側に入った後のチェックの方が強力ですよ。
（JS側は「入ってないよ？」という意思表示程度の対策として
　結果そのものを、重要視しない位置がいいと思います）

あと、
value="<?php echo $namae; ?>">
これは
value="<?= $namae ?>">
と略せます。

このソース、
PHPフィルタ前のソースなのは見ればわかりますが、
質問がJSという事で
function lengthCheck() {
これらが、サーバーサイドかクライアントサイドかで
戸惑う人はいるかも！（ただ、普通は見ればわかるが）
（大抵PHPの場合かなりの確率で、関数の2行目が$で始まるのでね）

おまけ！
this.dataset.submit_disabledについて
これ、関数の入り口で
this.dataset.submit_disabled = false;
にしてしまって、バリデーションOKのゲートのみでtrue
にするといいですよ。そうすると、
予期せぬ「抜け」で、falseが保証されるので、
問題が減ります。

あと、お金払って脆弱性診断受ける時、
大体は、PostMANスタイル（直接パラメタ捏造）なので
JavaScriptは実行すらさせてもらえなかったり＾＾

お礼

説明が不足しておりました、申し訳ありません。
PHP側でバリデーションと文字エスケープ、入力文字チェック、空白チェックは行い可能な限りXSSの対策はしたのですが、Javascriptのセキュリティ項目を調べた際にinnerHTMLがDOM-based XSSの発生源になると書いてあった為、心配になりJavascriptの方も対策しておくように考えておりました。

※参考サイト
https://gihyo.jp/dev/serial/01/javascript-security/0006

※PHP側でのXSS対策コード
function Chk_StrMode($str)
{
// タグを除去
// htmlspecialchars()を通したら、タグ判定されないため消去　$str = strip_tags($str);
// 空白を除去
$str = mb_ereg_replace('^(　){0,}', '', $str);
$str = mb_ereg_replace('(　){0,}$', '', $str);
$str = trim($str);
// 特殊文字を HTML エンティティに変換する
$str = htmlspecialchars($str);

return $str;
}
/* 未入力チェックファンクション */
function Chk_InputMode($str, $mes)
{
global $errors;
if ('' == $str) {
$errors[] = $mes;
}
}
/* 以下追加 */
function CheckUrl($checkurl, $mes)
{
global $errors;
if (preg_match("/[\.,:;]/u", $checkurl)) {
$errors[] = $mes;
}
}

補足

A.回答ありがとうございます、どうしても分からないのでアドバイスお願い致します。
名前(50文字)とメッセージ(500文字)欄にそれぞれ入力文字数をカウントして残り〇文字、〇文字超過と表示させるコードになるのですが、
innerHTML を使う場合は、たとえ問題のない値しか使わないと分かっていたとしても、HTMLのエスケープ処理を行うようにアドバイスを頂きコードを考えております、下記のコードを加えてみてはどうかとアドバイスを頂いたのですが、document.createElementをどのように使えばよいのか分からず困っております。デフォルトで残り字数を表示させて超過した場合字数をマイナスで表示させたい場合どのようにエスケープするべきでしょうか？

※名前HTML
<div class="msg_partial">あと<strong>50</strong>文字</div>

※コメントHTML
<div class="msg_partial">あと<strong>500</strong>文字</div>

※アドバイス内容
ブラウザの標準機能にエスケープ関数が無いので独自に用意しましょう
function escapeHTML(値) {
var a = document.createElement('span');
a.textContent = 値;
return a.innerHTML;
}
要素.innerHTML = '<strong>' + escapeHTML(文字列) + '</strong>';

※該当コード
<?php
class MAX_LENGTH
{
public const NAME = 50;
public const MESSAGE = 500;
}
?>

<script>
/* 名前とメッセージの文字数チェック */
const length_input = document.querySelectorAll('.length_input');
const submit_button = document.getElementById('submit_button');
for (let i = 0; i < length_input.length; i++) {
length_input[i].addEventListener('input', lengthCheck);
let event = new Event("input");
length_input[i].dispatchEvent(event);
}
function lengthCheck() {
const left = this.dataset.maxlength - this.value.length;
if (left >= 0) {
this.nextElementSibling.innerHTML = 'あと<strong>' + left + '</strong>文字';
this.dataset.submit_disabled = this.value.length === 0;
} else {
this.nextElementSibling.innerHTML = '<strong>' + -left + '</strong>文字超過しています';
this.dataset.submit_disabled = true;
}
let disabled = false;
for (let i = 0; i < length_input.length; i++) {
if (length_input[i].dataset.submit_disabled === "true") {
disabled = true;
}
}
submit_button.disabled = disabled;
}

回答No.1

t_ohta

return 文の後にいくら処理を書いても実行されません。
return 文を実行した時点で処理は関数の呼出元へ遷移してしまい、次行以降の処理は無視されます。

補足

回答ありがとうございます、勉強になりました。