move_uploaded_fileに関して困っています。

Question

        <?php
            if(isset($_FILES['file'])){
                
                $name = $_FILES['file']['name'];
                $type = explode('.',$name);
                $type = end($type);
                $size = $_FILES['file']['size'];
                $tmp = $_FILES['file']['tmp_name'];
                $random_name = rand();
                
                if($type != 'mp4' && $type != 'MP4'){
                    $message = "This format is not supported.";
                }else{
                    move_uploaded_file($tmp,'files/'.$random_name.'.'.$type);
                    try{
                        $pdo = new PDO('mysql:host=127.0.0.1; dbname=データベース名;','root');
                        $stmt = $pdo -> prepare ("INSERT INTO files VALUES('','$name','files/$random_name.$type')");
                        $stmt -> execute();
                    
                        $message = "Succesfully Uploaded.";
                        
                    }catch(PDOException $e){
                        exit('failed connecting to DB.'.$e -> getMessage());
                    }
                }
                
                echo "$message <br/><br/>";
            }
        ?>

このコードに関してですが、すべてちゃんと動いてくれてます。
ただ一つ困っているのが、

ファイルをセットし、サブミットしてからリロードするたびに動画ファイルが'files/'にアップロードされてしまいます。

これはこういうものなのでしょうか？それとも改良の余地があるのでしょうか？
また、この疑問が伝わるかが心配なので、わからないようであれば、ご指摘していただけるとありがたいです。
よろしくお願いします。

Answer 1

webアプリケーションって仕組み的にはそういうものです。
ファイルアップロードに限らず、あらゆるリクエストで同じことが発生します。

ただ、防止する方法はあって、
PHP 多重送信防止で検索すると色々と出てきます。

方向性としては

• 処理成功時に成功ページにリダイレクトさせることでリロードしてもリダイレクト部分がリロードされるようにする
• 処理時に処理内容を記憶しておいて（トークン等を使うことが多いです）、2回目は問答無用でエラーにする

という感じですね。
前者は簡易的な実装で、リダイレクトより前に戻るボタンで戻ってから進んだりして、同じリクエストを複数回送信すると登録できてしまうので、厳密に実装したいなら後者の方向性になります。

Comments

[退会済みユーザー]

とても勉強になります...！！
まだ初心者なので難しい用語はわからないですが、検索してみたら解決できました！
ありがとうございました。

Answer 2

質問に直接無関係な面もありますが，セキュリティ等改善すべき点がいくつかあるので，遅いながら回答させていただきます．

評価基準

★☆☆: よりよい書き方が考えられる
★★☆: エラーリスクがある
★★★: 脆弱性がある

★★☆ $_FILES['file']が存在していても期待した構造とは限らない

<input type="file" name="file">

から送信された場合，

$_FILES = [
    'file' => [
        'error' => ...,
        'tmp_name' => '...',
        ...
    ],
];

のような構造になりますが，HTTPリクエストはこのHTMLフォームから送信されるとは限りません．任意のプログラムから悪意のあるリクエストを飛ばすことも簡単にできます．そのため，例えば

<input type="file" name="file[x][y][z]">`

に相当するリクエストが飛んできたとしましょう．このとき，$_FILESの構造はこのようになってしまいます．

$_FILES = [
    'file' => [
        'error' => [
            'x' => [
                'y' => [
                    'z' => [
                        ...,
                    ],
                ],
            ],
        ],
        'tmp_name' => [
            'x' => [
                'y' => [
                    'z' => [
                        ...,
                    ],
                ],
            ],
        ],
        ...
    ],
];

こんなリクエストに対して具体的に「ここがおかしいよ！」とまで律儀なエラーメッセージを出す必要はありませんが，少なくともどんなリクエストがきてもサーバ側でエラーが発生しないようにコードを書くべきです．簡易的な解決策としては，issetのチェックを以下のように変更することが挙げられるでしょう．

if (isset($_FILES['file']) && is_int($_FILES['file']['error'])) {
    ...
}

より詳しい方法については「ファイルアップロードの例外処理はこれぐらいしないと気が済まない」でまとめています．

★★☆ アップロードのエラーはチェックする

上記の構造のチェックに加えて， $_FILES['file']['error'] が UPLOAD_ERR_OK になっているかどうかの確認も必要です．失敗している場合は move_uploaded_file のコールでエラーが発生するでしょう．

★☆☆ 拡張子を信じてはいけない

拡張子はあくまで参考程度の情報で，偽装されている場合やそもそもファイル名にドットが含まれていない場合すら考えられます．必ずしも中身までチェックする必要はないですが，簡易的にデータタイプをチェックしておきたいならばmime_content_type関数を使うとよいでしょう．

$mime = mime_content_type($_FILES['file']['tmp_name']); // "video/mp4"

但し…

• ファイルを設置するためにその拡張子を使うのであれば，拡張子のチェックは必須です．
• .phpなどアクセスするだけで何かが実行されてしまう拡張子は除外する必要がありますが，どちらかというと提示されているコードのように.mp4だけを認める，といった感じでホワイトリスト式にするほうが正解です．この点は評価できると思います．
• $_FILES['file']['type']にもMIMEタイプが含まれていますが，こちらはWebブラウザの自己申告に基づくものなので，信用できる値ではありません．

randは言うほどランダムではない

• せっかくアルファベットも使えるのに，数字だけにしてしまうとランダム性**(エントロピー)**が小さくなってしまいます．
• rand は乱数の精度が悪いと言われています．

ランダムなファイル名を作りたいのであれば，「ランダムなパスワードを1行で生成する with openssl」のようにするといいかと思います．

また，ファイルデータを削除不可，あるいは共有している人間がいなくなるまで実体の削除は行わないようにする場合，重複したアップロードで二重にファイルを作らせないこともできます．ファイルデータから一意なハッシュ値を取ってファイル名に使いましょう．

$hash = sha1_file($_FILES['file']['tmp_name']);

★★★ データベース関連の扱いが不適切

「PHPでデータベースに接続するときのまとめ」の「初心者がやりがちなミス」をご覧ください．

• データベースで使用する文字コードの指定をしていない
• "SELECT * FROM users WHERE id = '$id'" のように変数展開を使ってSQL文を組み立てている

Comments

[退会済みユーザー]

評価ありがとうございます。
時間がかかると思いますが、改善していきたいと思います。