View Issue Details

IDProjectCategoryView StatusLast Update
0023482FPCPackagespublic2013-01-04 22:27
ReporterAndrey M. Assigned ToFlorian  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
OSLinux 
Product Version2.6.0 
Fixed in Version3.0.0 
Summary0023482: TUnzipper cannot list and unpack archives > 2 GB
DescriptionWhen zip archive is larger than 2048 MB, code in attached file fails:

$ ./testzip test.zip
An unhandled exception occurred at $08066F3F :
EZipError : Corrupt ZIP file f.fb2-270105-275617.zip
  $08066F3F
  $08067D33
  $0804813E

When I remove some files from archive, program works correctly.
TagsNo tags attached.
Fixed in Revision23211,23230
FPCOldBugId0
FPCTarget
Attached Files

Activities

2012-12-13 01:16

 

testzip.pas (290 bytes)

Reinier Olislagers

2012-12-13 10:10

developer   ~0064273

Confirmed with FPC x86 trunk, Windows.

Problem appears to be here in zipper.pp (AEndHdrPos ends up being -1):
procedure FindEndHeader(AZip: TStream; out AEndHdr: End_of_Central_Dir_Type; out AEndHdrPos: Int64; out AZipFileComment: string);
var
  Buf: PByte;
  BufSize: Integer;
  I: Integer;
begin
  AZipFileComment := '';
  AEndHdrPos := AZip.Size - SizeOf(AEndHdr);
  if AEndHdrPos < 0 then
  begin
    AEndHdrPos := -1;
    FillChar(AEndHdr, SizeOf(AEndHdr), 0);
    exit;
  end;
  AZip.Seek(AEndHdrPos, soFromBeginning);
  AZip.ReadBuffer(AEndHdr, SizeOf(AEndHdr));
  {$IFDEF FPC_BIG_ENDIAN}
  AEndHdr := SwapECD(AEndHdr);
  {$ENDIF}
  if (AEndHdr.Signature = END_OF_CENTRAL_DIR_SIGNATURE) and
     (AEndHdr.ZipFile_Comment_Length = 0) then
    exit;

  // scan the last (64k + something) bytes for the END_OF_CENTRAL_DIR_SIGNATURE
  // (zip file comments are 64k max)
  BufSize := 65536 + SizeOf(AEndHdr) + 128;
  if AZip.Size < BufSize then
    BufSize := AZip.Size;

  Buf := GetMem(BufSize);
  try
    AZip.Seek(AZip.Size - BufSize, soFromBeginning);
    AZip.ReadBuffer(Buf^, BufSize);

    for I := BufSize - SizeOf(AEndHdr) downto 0 do
    begin
      if (Buf[I] or (Buf[I + 1] shl 8) or (Buf[I + 2] shl 16) or (Buf[I + 3] shl 24)) = END_OF_CENTRAL_DIR_SIGNATURE then
      begin
        Move(Buf[I], AEndHdr, SizeOf(AEndHdr));
        {$IFDEF FPC_BIG_ENDIAN}
        AEndHdr := SwapECD(AEndHdr);
        {$ENDIF}
        if (AEndHdr.Signature = END_OF_CENTRAL_DIR_SIGNATURE) and
           (I + SizeOf(AEndHdr) + AEndHdr.ZipFile_Comment_Length = BufSize) then
        begin
          AEndHdrPos := AZip.Size - BufSize + I;
          AZip.Seek(AEndHdrPos + SizeOf(AEndHdr), soFromBeginning);
          SetLength(AZipFileComment, AEndHdr.ZipFile_Comment_Length);
          AZip.ReadBuffer(AZipFileComment[1], Length(AZipFileComment));
          exit;
        end;
      end;
    end;

    AEndHdrPos := -1;

Marco van de Voort

2012-12-22 21:03

manager   ~0064412

I checked that code, and I don't see anything wrong with it. I fixed two other 64-bit file problems in (trunk) r23211. If one of you has an easy test setup, then please retest.

Reinier Olislagers

2012-12-23 00:04

developer   ~0064413

Nope that didn't fix it.

One thing that could be is that my test zip has the newer zip64 file format/header (an extension to of the old style zip header). IIRC zip64 allows archives larger than 4GB.
That might be worth implementing in any case...

Marco van de Voort

2012-12-23 00:58

manager   ~0064414

Ok. So more missing of functionality than an outright bugs.

ZIP64: http://www.pkware.com/documents/casestudies/APPNOTE.TXT

Reinier Olislagers

2012-12-24 11:34

developer   ~0064426

@Marco: don't know. A small test zip64 file now does not lead to the problem mentioned in 64273.
Furthermore we don't know what file format leads to the OP's problems.

For clarity, I raised a new bug 23533 about missing zip64 support with a zip64 test file and some more references.
Suggest leaving this bug open and let the OP report on it:
1. Has the problem been solved with your fixes in r23211?
2. Is it a zip64 file?

Andrey M.

2012-12-24 12:49

reporter   ~0064427

Tested with latest revision, same error.

Archive seems to be non-zip64. zipinfo says "minimum software version required to extract: 2.0" for each file in archive.

Marco van de Voort

2012-12-24 14:12

manager   ~0064428

Note that there might be separate zip64 markers in the end of file dir and in file entries. Don't trust third party detections 100%.

Afaik zip64 also contains the old endofdir marker, but has one additional one.

Andrey M.

2012-12-24 19:21

reporter   ~0064431

Last edited: 2012-12-26 12:00

I tried to debug procedure FindEndHeader and found that AZip.Seek returns -1 and does not seek to needed position. AZip.ReadBuffer reads data from start of archive file.

The bug is in TStream class, not in TUnzipper - some kind of integer overflow.

Created bug 0023539 with test program

Reinier Olislagers

2012-12-26 13:51

developer   ~0064456

Based on Andrey M's work, bug 0023539 and Michael Van Canneyt's response: attached patch changes all seek calls to int64 calls.

Please review and if ok, apply.

However, this still doesn't fix things.
In
Procedure TUnZipper.ReadZipDirectory;
  for i:=0 to EndHdr.Entries_This_Disk-1 do
    begin
    FZipStream.ReadBuffer(CentralHdr, SizeOf(CentralHdr));
calls
  procedure TStream.ReadBuffer(var Buffer; Count: Longint);

    begin
       if Read(Buffer,Count)<Count then
         Raise EReadError.Create(SReadError);
    end;
which raises the exception.

Andrey M.

2012-12-26 14:48

reporter   ~0064462

Last edited: 2012-12-26 14:57

I changed type of Central_File_Header_Type.Local_Header_Offset and End_of_Central_Dir_Type.Start_Disk_Offset from LongInt to Cardinal, and it worked.

May be, it needs to change signed integer to unsigned or int64 in other places.

2012-12-27 19:20

 

bigzip.diff (7,939 bytes)   
Index: packages/paszlib/src/zipper.pp
===================================================================
--- packages/paszlib/src/zipper.pp	(revision 23227)
+++ packages/paszlib/src/zipper.pp	(working copy)
@@ -32,15 +32,15 @@
 
 Type
    Local_File_Header_Type = Packed Record
-     Signature              :  LongInt;
+     Signature              :  LongInt; //4 bytes
      Extract_Version_Reqd   :  Word;
      Bit_Flag               :  Word;
      Compress_Method        :  Word;
      Last_Mod_Time          :  Word;
      Last_Mod_Date          :  Word;
      Crc32                  :  LongWord;
-     Compressed_Size        :  LongInt;
-     Uncompressed_Size      :  LongInt;
+     Compressed_Size        :  LongWord;
+     Uncompressed_Size      :  LongWord;
      Filename_Length        :  Word;
      Extra_Field_Length     :  Word;
    end;
@@ -48,7 +48,7 @@
   { Define the Central Directory record types }
 
   Central_File_Header_Type = Packed Record
-    Signature            :  LongInt;
+    Signature            :  LongInt; //4 bytes
     MadeBy_Version       :  Word;
     Extract_Version_Reqd :  Word;
     Bit_Flag             :  Word;
@@ -56,25 +56,25 @@
     Last_Mod_Time        :  Word;
     Last_Mod_Date        :  Word;
     Crc32                :  LongWord;
-    Compressed_Size      :  LongInt;
-    Uncompressed_Size    :  LongInt;
+    Compressed_Size      :  LongWord;
+    Uncompressed_Size    :  LongWord;
     Filename_Length      :  Word;
     Extra_Field_Length   :  Word;
     File_Comment_Length  :  Word;
     Starting_Disk_Num    :  Word;
     Internal_Attributes  :  Word;
-    External_Attributes  :  LongInt;
-    Local_Header_Offset  :  LongInt;
+    External_Attributes  :  LongWord;
+    Local_Header_Offset  :  LongWord;
   End;
 
   End_of_Central_Dir_Type =  Packed Record
-    Signature               :  LongInt;
+    Signature               :  LongInt; //4 bytes
     Disk_Number             :  Word;
     Central_Dir_Start_Disk  :  Word;
     Entries_This_Disk       :  Word;
     Total_Entries           :  Word;
-    Central_Dir_Size        :  LongInt;
-    Start_Disk_Offset       :  LongInt;
+    Central_Dir_Size        :  LongWord;
+    Start_Disk_Offset       :  LongWord;
     ZipFile_Comment_Length  :  Word;
   end;
 
@@ -205,9 +205,9 @@
     TableFull   :  Boolean;  { Flag indicating a full symbol table             }
     SaveByte    :  Byte;     { Output code buffer                              }
     BitsUsed    :  Byte;     { Index into output code buffer                   }
-    BytesIn     :  LongInt;  { Count of input file bytes processed             }
-    BytesOut    :  LongInt;  { Count of output bytes                           }
-    FOnBytes    : Longint;
+    BytesIn     :  LongWord;  { Count of input file bytes processed             }
+    BytesOut    :  LongWord;  { Count of output bytes                           }
+    FOnBytes    :  LongWord;
     Procedure FillInputBuffer;
     Procedure WriteOutputBuffer;
     Procedure FlushOutput;
@@ -308,7 +308,7 @@
     FEntries: TZipFileEntries;
     FZipping    : Boolean;
     FBufSize    : LongWord;
-    FFileName   :  String;         { Name of resulting Zip file                 }
+    FFileName   : String;         { Name of resulting Zip file                 }
     FFileComment: String;
     FFiles      : TStrings;
     FInMemSize  : Integer;
@@ -364,12 +364,12 @@
 
   TFullZipFileEntry = Class(TZipFileEntry)
   private
-    FCompressedSize: LongInt;
+    FCompressedSize: LongWord;
     FCompressMethod: Word;
     FCRC32: LongWord;
   Public
     Property CompressMethod : Word Read FCompressMethod;
-    Property CompressedSize :  LongInt Read FCompressedSize;
+    Property CompressedSize : LongWord Read FCompressedSize;
     property CRC32: LongWord read FCRC32 write FCRC32;
   end;
 
@@ -396,7 +396,7 @@
     FOnOpenInputStream: TCustomInputStreamEvent;
     FUnZipping  : Boolean;
     FBufSize    : LongWord;
-    FFileName   :  String;         { Name of resulting Zip file                 }
+    FFileName   : String;         { Name of resulting Zip file                 }
     FOutputPath : String;
     FFileComment: String;
     FEntries    : TFullZipFileEntries;
@@ -1190,9 +1190,9 @@
 Procedure TZipper.GetFileInfo;
 
 Var
-  F : TZipFileEntry;
+  F    : TZipFileEntry;
   Info : TSearchRec;
-  I       : Longint;
+  I    : LongWord;
 {$IFDEF UNIX}
   UnixInfo: Stat;
 {$ENDIF}
@@ -1331,7 +1331,7 @@
 Begin
    ACount := 0;
    CenDirPos := FOutStream.Position;
-   FOutStream.Seek(0,soFrombeginning);             { Rewind output file }
+   FOutStream.Seek(0,soBeginning);             { Rewind output file }
    HdrPos := FOutStream.Position;
    FOutStream.ReadBuffer(LocalHdr, SizeOf(LocalHdr));
 {$IFDEF FPC_BIG_ENDIAN}
@@ -1362,18 +1362,18 @@
      {$ENDIF}
        Local_Header_Offset := HdrPos;
        end;
-     FOutStream.Seek(0,soFromEnd);
+     FOutStream.Seek(0,soEnd);
      FOutStream.WriteBuffer({$IFDEF FPC_BIG_ENDIAN}SwapCFH{$ENDIF}(CentralHdr),SizeOf(CentralHdr));
      FOutStream.WriteBuffer(ZFileName[1],Length(ZFileName));
      Inc(ACount);
-     FOutStream.Seek(SavePos + LocalHdr.Compressed_Size,soFromBeginning);
+     FOutStream.Seek(SavePos + LocalHdr.Compressed_Size,soBeginning);
      HdrPos:=FOutStream.Position;
      FOutStream.ReadBuffer(LocalHdr, SizeOf(LocalHdr));
 {$IFDEF FPC_BIG_ENDIAN}
      LocalHdr := SwapLFH(LocalHdr);
 {$ENDIF}
    Until LocalHdr.Signature = CENTRAL_FILE_HEADER_SIGNATURE;
-   FOutStream.Seek(0,soFromEnd);
+   FOutStream.Seek(0,soEnd);
    FillChar(EndHdr,SizeOf(EndHdr),0);
    With EndHdr do
      begin
@@ -1438,7 +1438,7 @@
       else
         begin
         // Original file smaller than compressed file.
-        FInfile.Seek(0,soFromBeginning);
+        FInfile.Seek(0,soBeginning);
         FOutStream.CopyFrom(FInFile,0);
         end;
     finally
@@ -1676,7 +1676,7 @@
   S : String;
   D : TDateTime;
 Begin
-  FZipStream.Seek(Item.HdrPos,soFromBeginning);
+  FZipStream.Seek(Item.HdrPos,soBeginning);
   FZipStream.ReadBuffer(LocalHdr,SizeOf(LocalHdr));
 {$IFDEF FPC_BIG_ENDIAN}
   LocalHdr := SwapLFH(LocalHdr);
@@ -1713,7 +1713,7 @@
     FillChar(AEndHdr, SizeOf(AEndHdr), 0);
     exit;
   end;
-  AZip.Seek(AEndHdrPos, soFromBeginning);
+  AZip.Seek(AEndHdrPos, soBeginning);
   AZip.ReadBuffer(AEndHdr, SizeOf(AEndHdr));
   {$IFDEF FPC_BIG_ENDIAN}
   AEndHdr := SwapECD(AEndHdr);
@@ -1730,7 +1730,7 @@
 
   Buf := GetMem(BufSize);
   try
-    AZip.Seek(AZip.Size - BufSize, soFromBeginning);
+    AZip.Seek(AZip.Size - BufSize, soBeginning);
     AZip.ReadBuffer(Buf^, BufSize);
 
     for I := BufSize - SizeOf(AEndHdr) downto 0 do
@@ -1745,7 +1745,7 @@
            (I + SizeOf(AEndHdr) + AEndHdr.ZipFile_Comment_Length = BufSize) then
         begin
           AEndHdrPos := AZip.Size - BufSize + I;
-          AZip.Seek(AEndHdrPos + SizeOf(AEndHdr), soFromBeginning);
+          AZip.Seek(AEndHdrPos + SizeOf(AEndHdr), soBeginning);
           SetLength(AZipFileComment, AEndHdr.ZipFile_Comment_Length);
           AZip.ReadBuffer(AZipFileComment[1], Length(AZipFileComment));
           exit;
@@ -1763,7 +1763,7 @@
 Procedure TUnZipper.ReadZipDirectory;
 
 Var
-  i : LongInt;
+  i : LongWord; //todo: expand to 8 bytes when introducing zip64 format
   EndHdrPos,
   CenDirPos : Int64;
   NewNode   : TFullZipFileEntry;
@@ -1774,7 +1774,7 @@
   if EndHdrPos < 0 then
     raise EZipError.CreateFmt(SErrCorruptZIP,[FileName]);
   CenDirPos := EndHdr.Start_Disk_Offset;
-  FZipStream.Seek(CenDirPos,soFrombeginning);
+  FZipStream.Seek(CenDirPos,soBeginning);
   FEntries.Clear;
   for i:=0 to EndHdr.Entries_This_Disk-1 do
     begin
@@ -1820,7 +1820,8 @@
 Procedure TUnZipper.UnZipOneFile(Item : TFullZipFileEntry);
 
 Var
-  Count, Attrs: Longint;
+  Count: int64;
+  Attrs: Longint;
   ZMethod : Word;
   LinkTargetStream: TStringStream;
   OutputFileName: string;
bigzip.diff (7,939 bytes)   

Reinier Olislagers

2012-12-27 19:20

developer   ~0064500

Last edited: 2012-12-27 19:21

Zip specs appnote.txt version 6.3.3 indicate :
- central dir file header/relative offset of local header (= Local_Header_Offset)
- offset of start of central directory with respect to the starting disk number(=End_of_Central_Dir_Type.Start_Disk_Offset)
are 4 bytes
and
4.4.1.1 All fields unless otherwise noted are unsigned and stored in Intel low-byte:high-byte, low-word:high-word order.
4.4.1.1. above indicates e.g. End_of_Central_Dir_Type.Central_Dir_Size must also be unsigned

Have modified all relevant 4 byte record members (including those Andrey mentiones) from longint to longword (should perhaps be cardinal?)
Left attributes and signature as is.
Patch attached (incorporates previous patch which I deleted)
Test program now works with my large zip file.

The test in packages\paszlib\tests\tczipper.pp still works

Please review and apply if ok.

Florian

2012-12-27 20:50

administrator   ~0064502

Thanks for the patch, applied.

Issue History

Date Modified Username Field Change
2012-12-13 01:16 Andrey M. New Issue
2012-12-13 01:16 Andrey M. File Added: testzip.pas
2012-12-13 10:10 Reinier Olislagers Note Added: 0064273
2012-12-22 21:03 Marco van de Voort Note Added: 0064412
2012-12-23 00:04 Reinier Olislagers Note Added: 0064413
2012-12-23 00:58 Marco van de Voort Note Added: 0064414
2012-12-24 11:34 Reinier Olislagers Note Added: 0064426
2012-12-24 12:49 Andrey M. Note Added: 0064427
2012-12-24 14:12 Marco van de Voort Note Added: 0064428
2012-12-24 19:21 Andrey M. Note Added: 0064431
2012-12-26 12:00 Andrey M. Note Edited: 0064431
2012-12-26 13:51 Reinier Olislagers Note Added: 0064456
2012-12-26 13:52 Reinier Olislagers File Added: zipstream2GB.diff
2012-12-26 14:48 Andrey M. Note Added: 0064462
2012-12-26 14:57 Andrey M. Note Edited: 0064462
2012-12-27 19:19 Reinier Olislagers File Deleted: zipstream2GB.diff
2012-12-27 19:20 Reinier Olislagers File Added: bigzip.diff
2012-12-27 19:20 Reinier Olislagers Note Added: 0064500
2012-12-27 19:21 Reinier Olislagers Note Edited: 0064500
2012-12-27 20:50 Florian Fixed in Revision => 23230
2012-12-27 20:50 Florian Status new => resolved
2012-12-27 20:50 Florian Fixed in Version => 2.7.1
2012-12-27 20:50 Florian Resolution open => fixed
2012-12-27 20:50 Florian Assigned To => Florian
2012-12-27 20:50 Florian Note Added: 0064502
2013-01-04 22:27 Marco van de Voort FPCOldBugId => 0
2013-01-04 22:27 Marco van de Voort Fixed in Revision 23230 => 23211,23230