Q&A
セキュリティー対策が質問の趣旨ということでしたら、最低限、動作可能なコードを提示ください。【修正後】のコードは特に、Notice警告が出ているはずです。
PHPで（グローバル変数含む）変数を使ったリンクを以下のように使った場合のセキュリティー対策が知りたいです。
以下のように対策前と対策後を自分なりに書いてみました。
正しい書き方が知りたいです。
よろしくお願いします。
【対策前】 <?php while($row = $stmt -> fetch(PDO::FETCH_ASSOC)) { ?> htmlタグ省略... <? echo "<a href='xxxxxxxxx.php?id=".$row['id']."'>".$row['title']."</a>";?> <?php } ?> 【対策後（失敗）】 <?php while($row = $stmt -> fetch(PDO::FETCH_ASSOC)) { ?> htmlタグ省略... <? $id=htmlspecialchars(filter_input(INPUT_row,'id')); echo "<a href='xxxxxxxxx.php?id={$id}'>".$row['title']."</a>";?> <?php } ?> 【再修正（追加分）】 <?php while($row = $stmt -> fetch(PDO::FETCH_ASSOC)) { ?> htmlタグ省略... <? htmlspecialchars($row['id']); echo "<a href='xxxxxxxxx.php?id={$id}'>".$row['title']."</a>";?> <?php } ?>
すいません…。質問内容を一旦編集して【対策（失敗）後】に直し、動くコードを再構築して追加いたします。申し訳ありません。。
先ほど対応完了し、一応こちらの環境だとエラーなしで動作確認できました。この度は本当に申し訳ありませんでした。なお修正した件の場合はセキュリティーホールorそもそも構文ミスありますでしょうか。ありましたら先ほど同様ご指摘いただけると助かります。よろしくお願いします。
回答3件
0
明らかな間違いが
php
1 htmlspecialchars($row['id']); // なんのためにいれたの？ 2echo "<a href='xxxxxxxxx.php?id={$id}'>".$row['title']."</a>";?> // $id ってどこからきたの？
ですね。
php
1$id = htmlspecialchars($row['id']); 2echo "<a href='xxxxxxxxx.php?id={$id}'>".$row['title']."</a>";?>
セキュリティの面で言えば、省略されている部分に重要な点がありますので、サンプルコードを元に説明します。
#変数にHTMLタグが代入されている点
php
1<? echo "<a href='xxxxxxxxx.php?id=".$row['id']."'>".$row['title']."</a>";?>
このコードは動作はしますが、好ましくないと感じます。
この出力部分、後になって a タグではなく、span に変更したいということになった場合、面倒ではありませんか？
PHPの変数にHTMLタグを代入すると、メンテナンス性が落ちます。
ですので、私は以下のように記述します。
php
1<a href='xxxxxxxxx.php?id="<?= $row['id']?>"><?= $row['title'] ?></a>
#HTMLに埋め込みしている <?php } ?>
よくあるコード例としては、以下のようなもの。
html
1<table> 2 <tr> 3 <?php while($row = $stmt -> fetch(PDO::FETCH_ASSOC)) { ?> 4 <td> </td> 5 <td> </td> 6 <?php } ?> 7 </tr> 8</table>
while だけならいいですが、このwhile の中に if の条件分岐などが入ることは稀ではありません。
そのとき、どちらの } が、while の終了なのか if の終了なのかが判別しにくく、バグの元になりかねません.
while(): 〜 endwhile, if(): 〜 endif; , foreach () : 〜 endforeach; のように記述すると、みやすいコードになります。
html
1<table> 2 <tr> 3 <?php while($row = $stmt -> fetch(PDO::FETCH_ASSOC)) : ?> 4 <td> </td> 5 <td> </td> 6 <?php endwhile; ?> 7 </tr> 8</table>
#htmlspecialchars の使い方
htmlspecialchars は、第3引数まで指定し、本当に最後に出力するときだけ、利用しましょう。無用に二重エスケープしてしまうミスを防ぎます。
php
1htmlspecialchars($string, ENT_QUATES, 'utf-8');
#HTMLのなかにPHPのロジックを入れない
HTMLの中にデータベースの接続処理であったり、値の加工などがあると、後でバグを修正することが面倒です。
以下のような構造にして、HTMLとPHP部分を明確に分けるようにするとメンテナンスしやすくなります。
php
1<?php 2// PHPの処理 3?> 4<!DOCTYPE HTML> 5<html lang="ja"> 6 <head> 7 <meta charset="UTF-8"> 8 <title></title> 9 </head> 10 <body> 11 </body> 12</html>
#サンプルコード
コードを書く位置、書き方すべて意味があって以下のように書いています。「その意図はなんだろうか」と推測しながらコードを読んでみてください。その上でわからないことがあれば、コメントで聞いてください。
php
1<?php 2 3// デバッグ（開発）時は必ず記述する 4ini_set('display_errors', true); 5error_reporting(E_ALL); 6 7/** 8 * XSS対策 9 * @param string $string 10 * @return string 11 */ 12function h($string) 13{ 14 return htmlspecialchars($string, ENT_QUOTES, 'utf-8'); 15} 16 17try { 18 // DSNには必ず文字コードの指定をする 19 $dsn = 'mysql:host=localhost;dbname=test;charset=utf8;'; 20 $username = 'roor'; 21 $password = 'password'; 22 $pdo = new PDO($dsn, $username, $password); 23 24 // データベースのエラーを例外にする 25 $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); 26 27 // SQLの実行結果を連想配列で取り出す。 28 $pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC); 29 30 // プリペアドステートメントを宣言する 31 $sql = 'SELECT id, name FROM sample WHERE id = :id'; 32 $stmt = $pdo->prepare($sql); 33 34 // $_GET['id'] で取得しない 35 $id = filter_input(INPUT_GET, 'id'); 36 37 // 配列の初期化 38 $arr = []; 39 40 // パラメータをセットする 41 $arr[':id'] = $id; 42 43 // SQL 実行 44 $stmt->execute($arr); 45 46 // すべてのレコードを返す 47 $rows = $stmt->fetchAll(); 48} catch (Exception $e) { 49 $err = $e; 50} 51?> 52<!DOCTYPE HTML> 53<html lang="ja"> 54 <head> 55 <meta charset="UTF-8"> 56 <title></title> 57 </head> 58 <body> 59 <div> 60 <?php if (isset($err)) : ?> 61 <p><?= h($err->getMessage()); ?></p> 62 <?php endif; ?> 63 64 <?php if (0 < count($rows)): ?> 65 <table> 66 <?php foreach ($rows as $row) : ?> 67 <tr> 68 <td><?= h($row['id']); ?></td> 69 <td><?= h($row['name']); ?></td> 70 </tr> 71 <?php endforeach; ?> 72 </table> 73 <?php endif; ?> 74 </div> 75 </body> 76</html>
#追記
①// 配列の初期化 $arr = []; // パラメータをセットする $arr[':id'] = $id;
です。そもそもこういう処理自体初見に近いです。
この記述なら、みたことあるのではないでしょうか？
php
1$arr = array(); 2$arr[':id'] = $id;
php
1$arr = array();
と
php
1$arr = [];
は同じことを行なっています。 [] はPHP5.4以上で可能になった書き方です。
②html内の出力もすごいですね。ifで0以上なら（if(!empty)出力もあり？）…foreachでぶん回してhtml...（←特殊をhtmlに変換でしたっけ‥？）で出力でしょうか？？
（他にも細かいこと聞いたらきりがなさそうです。）
この if の条件式がないと、
html
1<table> 2</table>
という、HTMLとして不十分な出力になってしまいますよね。
画像を出力するときも
そうです。基本的に、PHPの「変数」を echo 及び print で出力するときにはすべて htmlspecialchars 関数を通します。
投稿2016/11/26 09:12
編集2016/11/26 12:03退会済みユーザー
総合スコア0
コメントありがとうございます。
いろいろな情報ありがとうございます。
まず１つ１つ確認させていただきます。
またこんなキレイなコードになるんですね！
ビックリです。
（個人的にはendifの使い方や無駄なく出力している点は参考にしたいです。がまだ理解できていないので練習でやってみます。）
とりあえず確認してわからないことがありましたら質問させてください。
よろしくお願いします！
お時間あるときで構いません（週明け移行でもOKです）のでお聞きしたいこといっぱいあります。
とりあえず「サンプルコード」でわかっている部分は
①try...catchで接続&例外エラー処理
②$stmt = $pdo->prepare($sql);セット？→ $stmt->execute($arr);発射（実行）？の流れ
あたりでしょうか。
ここら辺は（頭に自分でも）書いています。
逆にわからないのは
①// 配列の初期化 $arr = []; // パラメータをセットする $arr[':id'] = $id;
です。そもそもこういう処理自体初見に近いです。
②html内の出力もすごいですね。ifで0以上なら（if(!empty)出力もあり？）…foreachでぶん回してhtml...（←特殊をhtmlに変換でしたっけ‥？）で出力でしょうか？？
（他にも細かいこと聞いたらきりがなさそうです。）
ちょっと実装しているのですがこんな処理初めてなんで扱いきれていないです…。（実装自体は応用してできています。）
もしお時間ありましたら解説あると有り難いです！
休日にもかかわらず素晴らしい回答ありがとうございます。勉強になります！
画像を出力するときも
<tr>
<td><img src="xxxxxxxxxx.php?id=<?= h($row['id']); ?>">
</td>
</tr>
で<? h()....; ?>挟んで出力でしょうか？？
お疲れ様です。
教えていただいたコードで別途（複製）サイト作成しています。
そこでセキュリティー面でもう一つお聞きしたいのですが以下のようなコードでリンク受け渡す場合いつも$_GET（グローバル）で値を取っているのですがこれもいけないのでしょうか？？
何か回避するなどの処理必要でしょうか。
（日曜日ですのでお返事は月曜日でも問題ありませんのでよろしくお願いします。）
【index.php】メインページ
<?
接続処理上記参考...
?>
<html省略...>
<tr>
<td><a href="xxxxxxxx.php?id=<?= h($row['id']); ?>"><?= h($row['title']); ?></a></td>
</tr>
【xxxxxxx.php】個別ページ
<?
...
$sql = 'SELECT id, title , FROM xxxxxxx WHERE id='.$_GET['id'].'';
（わかりづらいですけどシングル＆ドットで囲っています‥。）
...
?>
<html省略..>
<tr>
<td><?= h($row['id']); ?></td>
</tr>
<tr>
<td><?= h($row['title']); ?></td>
</tr>
$sql = 'SELECT id, title , FROM xxxxxxx WHERE id='.$_GET['id'].'';
このコードは二つの意味でアウトです。
・SQLインジェクション脆弱性がある
・スーパーグローバル変数に直接アクセスしており、かつiindexの存在についての確認がない。
参考
http://junk-blog.com/php_filter_input/
回答ありがとうございます。
‥よくよく見たら前回のコードですでにグローバル変数の処理やっていましたね‥。
休日中にすいませんでした。。
全てを理解するのは難しいかもしれませんが、一般的にどんなことに注意すべきなのか、どんな風に設計するのかが以下の記事で雰囲気はつかめるかと思います。
PHPでログイン機能を実装するチュートリアル
http://qiita.com/ShibuyaKosuke/items/f114ffccf441edb2b745
お疲れ様です。
先日はありがとうございました。またログインの情報まで教えていただきありがとうございます。
前回の続きで申し訳ないのですがお聞きしたいことがあります。
例えば上記のコード＋別テーブルに入っているカテゴリーを表示する場合（カテゴリーA、カテゴリーB、カテゴリーC）も頭にロジックを書くのでしょうか？？
（includeやrequireで引っ張てくる感じでしょうか。）
現在
テーブルX＝id、title
テーブルY＝カテゴリーA、B、C
テーブルZ＝いろいろ...
があります。
今までは
<?
try{
DB接続..
}
catch{
..
}
?>
<html>...
<?
...select id,title from テーブルX;
echo $xxxx['id'];
echo $xxxx['title'];
?>
<?... select カテゴリー form テーブルY")
while($row = $stmt -> fetch(PDO::FETCH_ASSOC)) {
echo "<a href=>カテゴリー</a>";
?>
みたいな処理をしていました。
今回とは全く別物のため改めて組み直しているのすが…こういう場合前回のように頭に定義で追加＆接続処理でbody内で<foreach＞で回して<?php endforeach; ?>&<?php endif; ?>で終了でしょうか？？
よろしくお願いします
（誤字脱字ありましたらすいません。）
正解というものはありません。論理的に矛盾がない限りどんな形でも正解です。ただ、やはりPHPロジックとHTMLは分けて書く方がソースコードの見通しは良くなります。
カテゴリー一覧を取得する関数とか昨日単位で「関数」を定義するともっとスッキリしますね。
0
idの方はクエリ文字列の中に入るので、パーセントエンコード（URLエンコード）も必要です。
そうしないと、idが 1&a=1 のような値の場合、&a=1 以降が別のパラメータ(a=1)と認識されてしまいます。
このため、まず urlencode 関数等でパーセントエンコードし、その結果を htmlspecialchars でHTMLエスケープすべきです。該当箇所のみ最小限のコード例を示します。
PHP
1$idx = htmlspecialchars(urlencode($row['id']), ENT_QUOTES, 'UTF-8'); 2echo "<a href='xxxxxxxxx.php?id={$idx}'>". htmlspecialchars($row['title'], ENT_QUOTES, 'UTF-8') ."</a>";?>
投稿2016/11/26 13:20
総合スコア11701
0
idだけではなく、titleもエスケープすべきだと思いますよ
php
1<?php 2function h($str { 3 return htmlspecialchars($str, ENT_QUOTES, 'UTF-8') 4} 5?> 6<?php while($row = $stmt -> fetch(PDO::FETCH_ASSOC)): ?> 7 <a href='xxxxxxxxx.php?id=<?= h($row['id']) ?>'><?= h($row['title']) ?></a> 8<?php endwhile; ?>
※DBから取得した値や外部から取得した入力値などをHTMLに出力する場合は、原則すべてHTMLエスケープしたほうがいいと思います。
※htmlspecialchars()は長いので、h()関数を定義しておくのが普通です（Cakephpなどでもそうなっています）
※h()でエスケープした値は変数に入れると別の処理で間違って使ってしまう可能性もあるので、出力時に直接書く方がいいと思います。
投稿2016/11/26 11:09
総合スコア6586
icchiiさんコメントありがとうございます。
titile部分の件ご指摘ありがとうございます。
上記の処理も参考にさせていただきます。
にしてもPHPゴリゴリに書いていたのですがこんなキレイに書けるんですね‥。
初めて知りました‥。
自分のとは別物ですね。
icchiiさんに教えていただいた「html...→h()関数に置き換え」、「変数の混合？」は参考にします！
とりあえずちょっと情報がたくさんありすぎて整理します。
コメントありがとうございます！
古典的ですがHTMLエスケープもれを見つける方法として、入力欄に<script>alert(1)</script>を入れてみるといいと思いますよ。エスケープがもれているとダイアログが表示されます。お試しあれ。
ありがとうございます！！
ちなみに入力欄とは…？
例えばtitleを入力する画面があれば、titleに<script>alert(1)</script>と入力するという意味です。
ありがとうございます。やってみます！
あなたの回答
tips
プレビュー
