PHPの質問。動作させようとしている自サイトの検索機能があるのですが、どうもセキュリティに不安を感じています。どなたかチェックしていただけないでしょうか?

コードは文字制限で入らなかったので、コメント欄にあります。

回答の条件
  • 1人5回まで
  • 13歳以上
  • 登録:2010/04/09 15:46:31
  • 終了:2010/04/15 14:01:09

回答(5件)

id:Bombastus No.1

ホーエンハイム回答回数409ベストアンサー獲得回数522010/04/09 16:33:00

ポイント70pt

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

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

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

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

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

2010/04/11 15:14:00
id:ko8820 No.2

ko8820回答回数1221ベストアンサー獲得回数692010/04/09 19:25:26

ポイント20pt

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

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

id:tontonpokopoko

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

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

2010/04/11 15:09:59
id:gekikawa No.3

gekikawa回答回数110ベストアンサー獲得回数112010/04/10 13:04:49

ポイント70pt

はじめまして。

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

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


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


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

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

id:tontonpokopoko

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

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

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

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

時間を取られています。

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

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

2010/04/11 15:18:23
id:hanako393 No.4

hanako393回答回数1142ベストアンサー獲得回数872010/04/11 17:28:08

ポイント20pt

>$filedir='/image';

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

id:tontonpokopoko

やはりですか・・

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

2010/04/14 13:17:16
id:GreenStar No.5

GreenStar回答回数192ベストアンサー獲得回数462010/04/13 04:56:17

ポイント100pt

用意されている関数(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);
id:tontonpokopoko

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

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

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

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

2010/04/15 13:59:34
  • id:tontonpokopoko
    <?php

    # データ準備(ファイル検索)
    $filedir='/image';
    $dirlist=array();
    if($strDir = opendir($filedir)){
    while(false !== ($str = readdir($strDir))){
    array_push($dirlist, $str);
    }
    closedir($strDir);
    } else {
    print "ディレクトリを開けませんでした\n";
    }

    $pattern = '/key/i';
    $fl_array = array_values(preg_grep($pattern, $dirlist));

    print ("<hr>\n");

    # 配列に格納
    $result = array();
    $x = 0;
    $fl_count = count($fl_array);
    for($i=0; $i<$fl_count; $i++){
    $result[$x][] = $fl_array[$i];
    if ($i % 5 == 4) {
    $x++;
    }
    }

    print ("<hr>\n");

    if(isset($_GET['id'])){
    $p = $_GET['id'] - 1;
    } else {
    $p = 0;
    }

    $result_count = count($result[$p]);

    # 表示
    for($i=0; $i<$result_count; $i++){
    print ("<img src=\"/image/");
    print ($result[$p][$i]);
    print ("\">");
    }

    function pager($id,$fl_count){
    if($id=="") $id=1;

    $maxPage=ceil($fl_count/5);
    if($maxPage==1 or $maxPage<$id) return false;

    if($id>6){
    $startPage=$id-5;
    $startMore="<a href=\"$PHP_SELF?id=".($startPage -1)."\">&lt; PREV</a>";
    }else{
    $startPage=1;
    }

    if($id+5<$maxPage){
    $endPage=$id+5;
    $endMore=" <a href=\"$PHP_SELF?id=".($endPage+1)."\">NEXT &gt;</a>";
    }else{
    $endPage=$maxPage;
    }

    $page_footer="";
    for($i=$startPage;$i<=$endPage;$i++){
    $page_footer.=" ".(($id==$i)?"<span style='font-Size:120%'>$i</span>":"<a href=\"$PHP_SELF?id=$i\">$i</a>");
    }

    $page_footer=$startMore.$page_footer.$endMore;
    print $page_footer."<br>";
    }

    pager($_GET['id'],$fl_count);

    ?>

    <form method="get" action="<?php echo $SCRIPT_NAME ?>">
    <input type="submit" value="検索">
    </form>

この質問への反応(ブックマークコメント)

「あの人に答えてほしい」「この質問はあの人が答えられそう」というときに、回答リクエストを送ってみてましょう。

これ以上回答リクエストを送信することはできません。制限について

絞り込み :
はてなココの「ともだち」を表示します。
回答リクエストを送信したユーザーはいません