質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

87.61%

ファイルアップロード時に注意すべきこと

解決済

回答 2

投稿 編集

  • 評価
  • クリップ 0
  • VIEW 689

score 225

お世話になります。PHPでファイルをアップロードする仕組みを考えており、ここ3日ほど、色々なサイトで紹介されているスクリプトを拝見しました。

(ここteratailの他のスレッドでも書かれていましたが)注意すべき点として挙げるべき点は、

  1. 日本語がファイル名に含まれていないか?
  2. ファイル名にA-Za-z_-. 以外の文字が含まれていないか?
  3. POST時にエラーが発生していないか?
  4. ファイル名が空でないか?
  5. 不正なパスを表す文字列がファイル名に含まれていないか?
  6. ファイルサイズが0バイトより大きく指定値以内か?
  7. 本当にアップロードされたファイルなのか?
  8. 既存ファイルが存在していないか?
  9. ファイル名をハッシュ化して移動・保存。
  10. png,jpg,pdfなど拡張子を制限する。
  11. SWFファイルの制限。
  12. 保存フォルダを公開ディレクトリから外す

であるとまとめたわけですが、紹介されているサンプルコードは、簡素化したものか、もしくは私には難易度の高いもの両極端だったので、上記点を(12以外はカバーしてる?)踏まえたコードを作ってみました。

しかし、これで良いのか自信がなく、また身近に見てくれる人がいないため、ご指摘いただければ幸いと考え投稿しました。

何処かおかしなところがありましたらご指摘いただけないでしょうか?
ちなみに、アップを受付ける拡張子は、png,jpg,pdfの3種です。
よろしくお願いいたします。

<?php

$maxsize = $_POST["MAX_FILE_SIZE"];

$extension = array("image/jpeg", "image/png", "application/pdf");

$ds = DIRECTORY_SEPARATOR;

$storeFolder = "temp_dir";    //同じディレクトリ内にある一時保存用ディレクトリ

$DirPath = dirname( __FILE__ ).$ds.$storeFolder.$ds;

$CookieData = $_COOKIE["CookieData"];    //事前にユーザ毎に付与しているランダムな文字列

//[アップロード]ボタンの押下確認
if (isset($_POST["upload"])) {

    foreach($_FILES["upfile"]["tmp_name"] as $no => $tmp_name){

        //エラーはないか?
        if(!empty($_FILES["upfile"]["name"][$no]) && empty( $_FILES["upfile"]["error"][$no])){

        //サイズは超えていないか?
        if(($_FILES["upfile"]["size"][$no] > 0) && ($_FILES["upfile"]["size"][$no] < $maxsize)){

            //アップロードされたファイルか?
            if(is_uploaded_file($_FILES["upfile"]["tmp_name"][$no])){    //アップロードされたファイルを確認

            //拡張子を判定する
            $type = mime_content_type($_FILES["upfile"]["tmp_name"][$no]);
            if(in_array($type, $extension)){

                //一時保管されているファイル名
                $tempFile = $_FILES["upfile"]["tmp_name"][$no];

                //アップロードされたファイル名
                $filename = $_FILES["upfile"]["name"][$no];

                $file = explode(".", $filename);
                $ext = array_pop($file);

                //md5でハッシュ化。別のユーザーと同名のファイル名であったとしても被らないよう、
                //事前にユーザ別にランダムな文字列をcookieに持たせており、その値をファイル名へ。
                //また、ファイル数に応じて、番号も付与。
                //ファイル名を取得する際、basename()を用いることでファイルシステムトラバーサル攻撃は防げる
                //とのことだが、ハッシュ化すればその対策は必要ないのだろうか??

                $new_filename = $CookieData."-".$no.md5($filename).".".$ext;

                //設定するファイル
                $targetFile = $DirPath.$new_filename;

                //一時保管用にハッシュ化したファイルをセット
                if(move_uploaded_file($tempFile,$targetFile)){

                    //*** 画像ファイルだったらサイズ変更 ***
                    if($type != "application/pdf"){    

                        $upfile = $storeFolder."/".$new_filename;

                        list($width, $height, $mime_type) = getimagesize($upfile);

                        if($width > 1000){    //1000pxを超えるとき

                            $hiritsu = 1000 / $width;

                            $width_edit = floor($width * $hiritsu);
                            $height_edit = floor($height * $hiritsu);

                            switch($mime_type){

                                //jpegの場合
                                case IMAGETYPE_JPEG:
                                $src = imagecreatefromjpeg($upfile);
                                    break;

                                //pngの場合
                                case IMAGETYPE_PNG:
                                //拡張子の設定
                                $src = imagecreatefrompng($upfile);
                                    break;
                            }

                            $dst = imagecreatetruecolor($width_edit, $height_edit);

                            imagecopyresampled($dst, $src, 0, 0, 0, 0, $width_edit, $height_edit, $width, $height);

                            imagejpeg($dst, $upfile);

                        }

                    }//**********************

                        $error .= $no."成功です。<br />";
                }
                else{
                        $error .= $no."失敗です。<br />";
                }
            }
            else{
                $error .= $no."この拡張子は利用できません。<br />";
            }
            }
            else{
            $error .= $no."ファイルがアップロードされていません。<br />";
           }
        }
        else{
            $error .= $no."ファイルサイズが指定サイズを超えています。<br />";
        }
        }
        else{
        $error .= $no."ファイルアップロードエラーです。<br />";
        }
    }
}

?>
<html lang="ja">
<head>
<meta charset="utf-8">
<script>
<!--
//アップロードを許可する拡張子
var allow_exts = new Array('jpg', 'jpeg', 'png', 'pdf');

//フォーム内容の確認をする関数
function checkForm()
{
    var files = document.getElementsByName('upfile[]')[0].files;
    //ファイルが選択されているか確認
    if (files.length == 0) {
        alert('ファイルを選択してください');
        return false;
    }
    for (var i = 0; i < files.length; i++) {
        if (!checkExt(files[i].name)) {
            alert(files[i].name+'はアップロードできません');
            return false;
        }
    }
    return true;
}

function checkExt(filename)
{
    var ext = getExt(filename).toLowerCase();
    if (allow_exts.indexOf(ext) === -1) return false;
    return true;
}

function getExt(filename)
{
    var pos = filename.lastIndexOf('.');
    if (pos === -1) return '';
    return filename.slice(pos + 1);
}
-->
</script>
</head>
<body>

<h1>アップロード処理</h1>
<?php echo $error; ?>

<form action="./" enctype="multipart/form-data" method="post" onsubmit="return checkForm();">
<input type="hidden" name="MAX_FILE_SIZE" value="5097152">
<p><input type="file" name="upfile[]" accept=".jpg,.png,image/jpeg,image/png,application/pdf" /></p>
<p><input type="file" name="upfile[]" accept=".jpg,.png,image/jpeg,image/png,application/pdf" /></p>
<p><input type="file" name="upfile[]" accept=".jpg,.png,image/jpeg,image/png,application/pdf" /></p>
<p><input type="submit" name="upload" value="アップロード"></p>
</form>

</body>
</html>

■補足説明

皆様からのご指摘、真摯に受け止めたく思います。
今回の質問に至った経緯を改めてお伝えすると、今回、ファイルアップロードを調べているうちに、色々な危険性があることを知りました。

はじめの方は、何か良いサンプルコードはないものか?そんな気持ちで居ましたが、当然とは思いながらも初心者向けのものが多いのが実情。その反面、高度な内容のものもありましたが、私には難易度が高かったため、自分で理解をしながらコードを書いてみよう、というのが事の経緯です。

注意点として書き出したのは、
https://teratail.com/questions/30411
こちらをはじめ、自身で書き出してみようと考えまとめたもの。(このURLで書かれていることが半数以上を占めていますが・・)

コードに関しては、あえてシンプルなものが理想と考え、
https://blog.ver001.com/php-upload-multiple/
から拝借し、注意点を考慮しながら自身で書き加えたものです。
(コード中にあるコメントは自身の覚書のためのものです)

動作確認はしており、拡張子の振り分けをはじめ、きちんと設置したいフォルダに描いている形でリネーム、そしてアップロードが出来るまでは確認していますが、注意点として挙げているものの、対策として不十分なものがあるんじゃないか?と半信半疑だったことも手伝い、その他、手元の環境(というか知識がないため)、"./"などを含んだファイル名など用意できず、注意点として挙げた項目をどれだけ対策できているかアドバイスを仰ぎたいと考えての投稿でした。

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 過去に投稿した質問と同じ内容の質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

質問への追記・修正、ベストアンサー選択の依頼

  • 退会済みユーザー

    退会済みユーザー

    2020/05/06 11:42

    テスト仕様書を作ってテストを実施する、じゃぁテスト仕様書ってどう書くのかというと、どういう条件でどういうことを行えばこういう結果になるってことのまとめです。

    キャンセル

  • 退会済みユーザー

    退会済みユーザー

    2020/05/06 13:27

    ネストが多すぎて可読性が悪いので、関数を使って書いてみては?

    キャンセル

  • m.ts10806

    2020/05/06 14:52

    >挙げた項目をどれだけ対策できているか
    やはりテストケース作って自分でテストするのが一番でしょうね。
    こちらもコードだけで全て見えるわけではないので、結局テストケースを作る必要がありますが、そもそもそれって「要件」なので作る側が決めなければなりません。

    キャンセル

回答 2

check解決した方法

0

m.ts10806さん
AkitoshiManabeさん
m6uさん
Kosuke_Shibuyaさん

貴重なご意見、並びにご指摘、ご親切な書き込みをありがとうございました。
m6uさん、m.ts10806さんのコメントで腑に落ちました。

改めて、アップするファイルの取扱いを見直し、検討してみたいと思います。

コメントへ書き込みいただいた、yambejpさん含め、貴重なご意見に感謝しています。ありがとうございました。

※こちらで回答の受付を終了させると自身がベストアンサーになるのですね。違う気がしますが、終了とするため投稿いたします。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

0

  • アップされたファイル名をそのまま公開用に使うか
  • ファイル名はサーバー側で変更し、ダウンロード時にローダーによってファイルを
    つけ直すか

によって処理が違います
またDBで管理するかどうかでも変わってきます

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2020/05/07 11:16

    yambejpさん

    貴重なアドバイスをありがとうございます。

    他の方からもテスト仕様書を用意する旨伺っていますが、目的に応じて処理も違ってくるとのことですね。

    見落としがちな事が沢山出てきます。
    貴重なご意見ありがとうございました。参考にさせて頂きます。

    キャンセル

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 87.61%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

関連した質問

同じタグがついた質問を見る