人力検索はてな
モバイル版を表示しています。PC版はこちら
i-mobile

PHPの質問。動作させようとしている自サイトの検索機能があるのですが、どうもセキュリティに不安を感じています。どなたかチェックしていただけないでしょうか?
コードは文字制限で入らなかったので、コメント欄にあります。

●質問者: tontonpokopoko
●カテゴリ:ウェブ制作
✍キーワード:PHP コメント欄 コード サイト セキュリティ
○ 状態 :終了
└ 回答数 : 5/5件

▽最新の回答へ

1 ● ホーエンハイム
●70ポイント

気がついた点を2つほど挙げます。

  1. コマンドラインのidに文字列、正の整数以外の値、定義域を超えた値を代入されるとエラーが起こります。そのエラーにどう対処するかで、セキュリティレベルが変わってきます。
  2. formのactionで$SCRIPT_NAMEを参照させていますが、なぜaタグでは$PHP_SELFを参照しているのでしょうか。$SCRIPT_NAMEに統一した方がセキュリティリスクは低減されます。
◎質問者からの返答

回答をありがとうございます。$SCRIPT_NAMEに統一、

そして文字列の種類、長さなどを確認することでセキュリティを強化しました。

考え方すら分かっていないレベルなので、とても参考になりました。


2 ● ko8820
●20ポイント

>print "ディレクトリを開けませんでした\n";

エラーの内容がわかるのはまずいかと。

◎質問者からの返答

回答をありがとうございます。

エラーはとりあえずだったので、変更いたしました。


3 ● gekikawa
●70ポイント

はじめまして。

1.の回答者様のいうように、入力値のチェックがもうちょっとあった方がよいかなぁという点と

PHP_SELFはXSSの脆弱性のために使わない方が良いという点がきになります。(でも参考書には多いですねPHP_SELF)


ただ、それよりも$PHP_SELFと書かれているのでregister_globalsがONになっているのでしょうか?


デメリット(リスク)を承知の上ならばもちろん問題はないと思いますが、そうでなければ$_SERVER['SCRIPT_NAME']など

スーパーグローバル変数をお使いになった方が良いと思います。

◎質問者からの返答

はじめまして、回答をありがとうございます。

入力値のチェックという言葉で、いろいろと検索しまして、

いくつかセキュリティレベルを上げられたようです。

ただ今度は検索キーワードの代入、ページングの不都合などに直面して、

時間を取られています。

もう少し考えてみてダメなようなら、再びはてなで質問するかもしれません。

そのときはどうぞよろしくお願いいたします。


4 ● hanako393
●20ポイント

>$filedir='/image';

推測されやすい文字列は危険です。

◎質問者からの返答

やはりですか・・

少しひねったほうがいいのでしょうか・・


5 ● GreenStar
●100ポイント

用意されている関数(php5で軽くテスト済みphp4には無い関数を使っています)を使ってコードを短くすることもバグ対策/セキュリティ対策に有効です

・ファイル一覧処理を簡略化

・5つずつ配列に分けていた部分を省略してメモリ節約/処理時間節約

・3項演算子で行数節約(3項演算子をネストしたりするとコードが見辛くなるので無理して短縮しないように注意)

・id指定ミスの自動修正

おまけ

・管理者用エラー番号を出力させることによって、管理者のみエラー箇所が判る仕組み追加

・1ページあたりの表示数指定を追加

$message = 'エラーが発生しました。時間をおいて再度お試しください。管理者用エラー番号:';
$page = 5;

$filedir = '/home/ユーザー名/public_html/'; #URLではなくサーバ上の位置指定が必要。これは一例
$pattern = '/key/i';
$dirlist = @scandir($filedir) or die($message . '1');
$fl_array = @array_values(preg_grep($pattern, $dirlist)) or die($message . '2');

$p = (isset($_GET['id']) ? min(max(0, $_GET['id'] - 1), floor((count($fl_array) - 1) / $page)) : 0);

for($i=0; $i < $page; $i++){
 echo (isset($fl_array[$page * $p + $i]) ? '<img src="/image/' . $fl_array[$page * $p + $i] . '">' : '');
}

function pager($id,$fl_count){
以下省略

@ はphpからの直接的なエラーメッセージを抑制するためのものです。最初から出ないサーバもありますのでワザとエラーが出るようにして@ありと@なしでエラーメッセージに変化があるかどうかを確認してみてください。


3項演算子部分は下記と同じ意味になっています。function pagerの中でも同様に使えばスッキリするかもしれません。

if (isset($_GET['id'])) {
 $p = min(max(0, $_GET['id'] - 1), floor((count($fl_array) - 1) / $page));
} else {
 $p = 0;
}

min(max(0, $_GET['id'] - 1), floor((count($fl_array) - 1) / $page)) は下記の要領で処理されます。

1.$_GET['id'] - 1 指示されたページの修正
2.上記で指示されたページがゼロ以下にならないよう調整(max)
3.floor((count($fl_array) - 1) / $page)) ファイル数からページ数計算
4.現実よりページ数が大きくなりすぎないよう調整(min)

調整不要ならば下記で以前通りですが「文字列の種類、長さなどを確認することでセキュリティを強化しました」とのことなので適宜対応してください。

$p = (isset($_GET['id']) ? $_GET['id'] - 1 : 0);
◎質問者からの返答

回答をありがとうございました。

初期段階からキーワード検索(チェックボックス)やリファラ、文字列チェックなどを取り入れたため、改造が難しいみたいです・・

とりあえずはてなの期限が切れますので、

のちほどまた参考にさせていただきます。ありがとうございました。

関連質問


●質問をもっと探す●



0.人力検索はてなトップ
8.このページを友達に紹介
9.このページの先頭へ
対応機種一覧
お問い合わせ
ヘルプ/お知らせ
ログイン
無料ユーザー登録
はてなトップ