From: Nobuyoshi Nakada <nobu@...>
Date: 2008-02-06T11:56:06+09:00
Subject: [ruby-dev:33609] Re: cgi.rb再構築案

なかだです。

At Wed, 6 Feb 2008 02:06:34 +0900,
Makoto Kuwata wrote in [ruby-dev:33606]:
> CGI::Cookieを、DelegateClass(Array) を使わないように
> するためのpatch (for 1.8.6 p111) です。

1.9ではDelegateClass(Array)はかなり改善したと思います。

> 続いて以下の問題に対するpatch (for 1.8.6 p.111) です。
> [ruby-list:44321] で投稿したのと同じですが再送します。
> 
> * multipart時およびPOSTメソッド時に受信するデータのサイズを
>   確認していないため、巨大なデータを送信されるとサーバ資源が
>   食いつぶされる。
>   → Content-Lengthをチェックし、長過ぎれば例外を発生させる。
> 
> * multipart時に任意の数のparameterを受け付けるため、
>   例えば数千個のparameterを送信することで、サーバにたくさんの
>   Tempfileオブジェクトを生成することができてしまう。
>   → parameterの数をチェックし、多すぎれば例外を発生させる。

StandardErrorを使っていますが、別のほうがいいんじゃないでしょう
か。RangeError?

> 簡単化のため制限値は定数で指定してますが、必要であればクラス変数や
> インスタンス変数で指定できるようにしてください。

後者にしてみました。[ruby-dev:33606]からの差分にしようかと思いま
したが、パッチが壊れているようなのでsvn HEADからの差分です。

他に、NEEDS_BINMODEの判定条件がdarwinも含んでしまっている点や、
env_tableを使わずに直接ENVを使っているところも修正しています。

At Mon, 4 Feb 2008 04:40:16 +0900,
Makoto Kuwata wrote in [ruby-dev:33580]:
> (6) CGI#initialize() の中で ENV['QUERY_STRING'] や ENV['HTTP_COOKIE'] を
>     parse しているので、FastCGI のように ENV を使わない場合、
>     cgi.rb を使うのが困難
>     (実際、fcgi.rb での cgi.rb 対策は悲惨なコードになっている)
>     → 解決策: workaround として、$ENV = ENV をしてから、
>        CGI#initialize() では ENV のかわりに $ENV を使うように変更

これに関しては、CGIを継承したクラスでenv_tableをオーバーライドす
るのが正しいのではないでしょうか。たしかに漏れている部分もありま
したが、その二つについては以前からenv_tableを使うようになってい
ます。


Index: lib/cgi.rb
===================================================================
--- lib/cgi.rb	(revision 15377)
+++ lib/cgi.rb	(working copy)
@@ -287,5 +287,5 @@ class CGI
   REVISION = '$Id$' #:nodoc:
 
-  NEEDS_BINMODE = true if /WIN/ni.match(RUBY_PLATFORM) 
+  NEEDS_BINMODE = true if /cygwin|mswin|bccwin/.match(RUBY_PLATFORM)
 
   # Path separators in different environments.
@@ -737,5 +737,4 @@ class CGI
   end
 
-  require "delegate"
 
   # Class representing an HTTP cookie.
@@ -772,5 +771,5 @@ class CGI
   #   cookie1.expires = Time.now + 30
   #   cookie1.secure  = true
-  class Cookie < DelegateClass(Array)
+  class Cookie
 
     # Create a new CGI::Cookie object.
@@ -808,5 +807,5 @@ class CGI
         @path = options["path"]
       else
-        %r|^(.*/)|.match(ENV["SCRIPT_NAME"])
+        %r|^(.*/)|.match(env_table["SCRIPT_NAME"])
         @path = ($1 or "")
       end
@@ -859,4 +858,34 @@ class CGI
     end
 
+    ##--
+    ## define methods instead of DelegateClass(Array)
+    ##++
+
+    include Enumerable  ##:nodoc:
+
+    def [](*args)  ##:nodoc:
+      @value[*args]
+    end
+
+    def []=(index, value)  ##:nodoc:
+      @value[index] = value
+    end
+
+    def each(&block)  ##:nodoc:
+      @value.each(&block)
+    end
+
+    def method_missing(m, *args)  ##:nodoc:
+      @value.respond_to?(m) ? @value.__send__(m, *args) : super
+    end
+
+    def respond_to?(m)  ##:nodoc:
+      super(m) || @value.respond_to?(m)
+    end
+
+    #def inspect;  @value.inspect;  end
+    #def ==(arg);  @value == arg;  end
+    #def ===(arg);  @value === arg;  end
+
   end # class Cookie
 
@@ -908,4 +937,17 @@ class CGI
   end
 
+  # Maximum content length of post data
+  MAX_CONTENT_LENGTH  = 2 * 1024 * 1024
+
+  # Maximum content length of multipart data
+  MAX_MULTIPART_LENGTH  = 128 * 1024 * 1024
+
+  # Maximum number of request parameters when multipart
+  MAX_MULTIPART_COUNT = 128
+
+  # Maximum size to use StringIO
+  MAX_STRINGIO_SIZE = 10240
+
+
   # Mixin module. It provides the follow functionality groups:
   #
@@ -985,7 +1027,11 @@ class CGI
       end
 
+      count = @max_multipart_count
       loop do
+        unless (count -= 1) >= 0
+          raise RangeError, "too many parameters"
+        end
         head = nil
-        if 10240 < content_length
+        if content_length > @max_stringio_size
           require "tempfile"
           body = Tempfile.new("CGI")
@@ -1107,5 +1153,9 @@ class CGI
         boundary = $1.dup
         @multipart = true
-        @params = read_multipart(boundary, Integer(env_table['CONTENT_LENGTH']))
+        content_length = Integer(env_table['CONTENT_LENGTH'])
+        unless content_length <= @max_multipart_length
+          raise RangeError, "too large multipart data"
+        end
+        @params = read_multipart(boundary, content_length)
       else
         @multipart = false
@@ -1120,5 +1170,9 @@ class CGI
                     when "POST"
                       stdinput.binmode if defined? stdinput.binmode
-                      stdinput.read(Integer(env_table['CONTENT_LENGTH'])) or ''
+                      content_length = Integer(env_table['CONTENT_LENGTH'])
+                      unless content_length <= @max_content_length
+                        raise RangeError, "too large post data"
+                      end
+                      stdinput.read(content_length) or ''
                     else
                       read_from_cmdline
@@ -2263,9 +2317,13 @@ class CGI
   # CGI locations, which varies according to the REQUEST_METHOD.
   def initialize(type = "query")
-    if defined?(MOD_RUBY) && !ENV.key?("GATEWAY_INTERFACE")
+    if defined?(MOD_RUBY) && !env_table.key?("GATEWAY_INTERFACE")
       Apache.request.setup_cgi_env
     end
 
     extend QueryExtension
+    @max_content_length ||= MAX_CONTENT_LENGTH
+    @max_multipart_length ||= MAX_MULTIPART_LENGTH
+    @max_multipart_count ||= MAX_MULTIPART_COUNT
+    @max_stringio_size ||= MAX_STRINGIO_SIZE
     @multipart = false
     if defined?(CGI_PARAMS)


-- 
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦