Ticket #566 (closed バグ指摘: 修正済)

Opened 11 years ago

Last modified 10 years ago

CSVアップロードで無限ループの可能性

Reported by: nanasess Owned by: nanasess
Priority: Milestone: EC-CUBE2.4.3
Component: 管理画面 Version: 2.4.2
Keywords: Cc:
修正済み:

Description

CSVアップロード時, ファイルポインタチェックの不備により, 無限ループの可能性がある

#331 に類似

Change History

comment:1 Changed 11 years ago by nanasess

  • Owner changed from somebody to nanasess
  • Status changed from new to assigned

とり急ぎ, comu-ver2 に対する修正パッチです.

Index: LC_Page_Admin_Products_UploadCSVCategory.php
===================================================================
--- LC_Page_Admin_Products_UploadCSVCategory.php	(revision 18367)
+++ LC_Page_Admin_Products_UploadCSVCategory.php	(working copy)
@@ -108,6 +108,10 @@
 
                     // レコード数を得る
                     $rec_count = $this->lfCSVRecordCount($enc_filepath);
+                    if ($rec_count === false) {
+                        $err = false;
+                        $arrErr['bad_file_pointer'] = "※ 不正なファイルポインタが検出されました";
+                    }
 
                     $fp = fopen($enc_filepath, "r");
                     $line = 0;      // 行数
@@ -361,14 +365,18 @@
      * CSVのカウント数を得る.
      *
      * @param string $file_name ファイルパス
-     * @return integer CSV のカウント数
+     * @return mixed CSV のカウント数; $file_name が無効な場合は false
      */
     function lfCSVRecordCount($file_name) {
         $count = 0;
         $fp = fopen($file_name, "r");
-        while(!feof($fp)) {
-            $arrCSV = fgetcsv($fp, CSV_LINE_MAX);
-            $count++;
+        if ($fp !== false) {
+            while(!feof($fp)) {
+                $arrCSV = fgetcsv($fp, CSV_LINE_MAX);
+                $count++;
+            }
+        } else {
+            return false;
         }
 
         return $count-1;

comment:2 Changed 11 years ago by kajiwara

  • Status changed from assigned to closed
  • Resolution set to 修正済

nanasess様 本件、ご報告とご対応いただき、ありがとうございます。

r18409にて修正させていただきました。

本件は、無限ループが実際に発生したといった報告はありませんが、確かにソース上でエラーチェックがなく、不正ファイルがアップロードされた場合などは、危険かと思います。
(実際は不正ファイルがアップロードされても上記ループにまでたどり着くことはなく、その手前のエラーチェックに引っかかります。本件はそちらを仮にすり抜けてしまった場合の対応とお考えいただければと思います。)

カテゴリCSVファイルのアップロードを運用で行われる可能性がある場合は、本修正を適用されることを推奨します。

comment:3 Changed 11 years ago by nanasess

  • Status changed from closed to reopened
  • Resolution 修正済 deleted

comment:4 Changed 11 years ago by nanasess

  • Version changed from 2.4.1 to 2.4.2
  • Milestone changed from EC-CUBE2.4.2 to EC-CUBE2.4.3

この前のロジックでも, 同様の問題が発生します.

version-2_4-dev r18477 にて修正しました.

comment:5 Changed 11 years ago by nanasess

lfCSVRecordCount 関数と, その後の fopen の処理が冗長なので, 根本的にリファクタリングした方が良さそうです.

comment:6 Changed 11 years ago by nanasess

version-2_4-dev r18501 にて修正

comment:7 Changed 11 years ago by nanasess

comu-ver2 r18508 で修正

comment:8 Changed 11 years ago by nanasess

  • Status changed from reopened to closed
  • Resolution set to 修正済

version-2_4 r18525 で修正

comment:9 Changed 10 years ago by Seasoft

コミュニティ版は r18462 も関連しているようです。(備忘録)

Note: See TracTickets for help on using tickets.