check size of actual zip contents against quota

because the zip central directory can lie

test plan:
 1. Create a huge file (say a gigabyte) where every byte is the
    same.
 2. Zip it. The repeating data will be compressed. The ZIP will
    be tiny but will uncompress to a huge file.
 3. Edit the zip file binary. Open it with a hex editor and
    change the file size in the directory. Make the file
    appear to be small enough to fit into the course quota.
    (The zip file format is documented at
    http://www.pkware.com/documents/casestudies/APPNOTE.TXT
    or ask the committer for help)
 4. Try to import the zip file into a course (migrations/
    import zip file into folder)
 5. You should receive a quota error.

fixes CNVS-10722

Change-Id: Ib1bd1c432ef900f0c6c61ebe6eab2881f8515104
Reviewed-on: https://gerrit.instructure.com/29704
Reviewed-by: Mark Severson <markse@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: Nathan Rogowski <nathan@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>

lib/unzip_attachment.rb

@@ -113,7 +113,7 @@ class UnzipAttachment
         # have to worry about what this name actually is.
         Tempfile.open(filename) do |f|
           begin
-            entry.extract(f.path) { true }
+            extract_entry(entry, f.path)
             # This is where the attachment actually happens.  See file_in_context.rb
             attachment = attach(f.path, entry, folder)
             id_positions[attachment.id] = path_positions[entry.name]
@@ -121,6 +121,9 @@ class UnzipAttachment
               attachment.update_attribute(:migration_id, migration_id)
             end
             @attachments << attachment if attachment
+          rescue Attachment::OverQuotaError
+            f.unlink
+            raise
           rescue => e
             @logger.warn "Couldn't unzip archived file #{f.path}: #{e.message}" if @logger
           end
@@ -135,6 +138,18 @@ class UnzipAttachment
     update_progress(1.0)
   end
 
+  def extract_entry(entry, dest_path)
+    ::File.open(dest_path, "wb") do |os|
+      entry.get_input_stream do |is|
+        entry.set_extra_attributes_on_path(dest_path)
+        buf = ''
+        while buf = is.sysread(::Zip::Decompressor::CHUNK_SIZE, buf)
+          os << buf
+          zip_stats.charge_quota(buf.size)
+        end
+      end
+    end
+  end
 
   def zip_stats
     @zip_stats ||= ZipFileStats.new(filename)
@@ -246,13 +261,14 @@ end
 #since it's such an integral part of the unzipping 
 #process
 class ZipFileStats
-  attr_reader :file_count, :total_size, :paths, :filename
+  attr_reader :file_count, :total_size, :paths, :filename, :quota_remaining
 
   def initialize(filename)
     @filename = filename
     @paths = []
     @file_count = 0
     @total_size = 0
+    @quota_remaining = nil
     process!
   end
 
@@ -262,12 +278,27 @@ class ZipFileStats
       raise ArgumentError, "Zip File cannot have more than #{max} entries"
     end
 
+    # check whether the nominal size of the zip's contents would exceed
+    # quota, and reject the zip immediately if so
     quota_hash = Attachment.get_quota(context)
-    if quota_hash[:quota] > 0 && (quota_hash[:quota_used] + total_size) > quota_hash[:quota]
-      raise Attachment::OverQuotaError, "Zip file would exceed quota limit"
+    if quota_hash[:quota] > 0
+      if (quota_hash[:quota_used] + total_size) > quota_hash[:quota]
+        raise Attachment::OverQuotaError, "Zip file would exceed quota limit"
+      end
+      @quota_remaining = quota_hash[:quota] - quota_hash[:quota_used]
     end
   end
 
+  # since the central directory can lie, track quota during extraction as well
+  # to prevent zip bomb denial-of-service attacks
+  def charge_quota(size)
+    return if @quota_remaining.nil?
+    if size > @quota_remaining
+      raise Attachment::OverQuotaError, "Zip contents exceed course quota limit"
+    end
+    @quota_remaining -= size
+  end
+
   def paths_with_positions(base)
     positions_hash = {}
     paths.sort.each_with_index{|p, idx| positions_hash[p] = idx + base }

spec/lib/unzip_attachment_spec.rb

@@ -160,6 +160,37 @@ describe UnzipAttachment do
       end
     end
 
+    describe 'zip bomb mitigation' do
+      # unzip -l output for this file:
+      #  Length     Date   Time    Name
+      # --------    ----   ----    ----
+      #       12  02-05-14 16:03   a
+      #       18  02-05-14 16:03   b
+      #       70  02-05-14 16:05   c   <-- this is a lie.  the file is really 10K
+      #       19  02-05-14 16:03   d
+      let(:filename) { fixture_filename('zipbomb.zip') }
+
+      it 'double-checks the extracted file sizes in case the central directory lies' do
+        Attachment.stubs(:get_quota).returns({:quota => 5000, :quota_used => 0})
+        lambda{ unzipper.process }.should raise_error(Attachment::OverQuotaError)
+        # a and b should have been attached
+        # but we should have bailed once c ate the remaining quota
+        @course.attachments.count.should eql 2
+      end
+
+      it "doesn't interfere when the quota is 0 (unlimited)" do
+        Attachment.stubs(:get_quota).returns({:quota => 0, :quota_used => 0})
+        lambda{ unzipper.process }.should_not raise_error
+        @course.attachments.count.should eql 4
+      end
+
+      it "lets incorrect central directory size slide if the quota isn't exceeded" do
+        Attachment.stubs(:get_quota).returns({:quota => 15000, :quota_used => 0})
+        lambda{ unzipper.process }.should_not raise_error
+        @course.attachments.count.should eql 4
+      end
+    end
+
   end
 
   context "scribdable files" do