Fixed #46: Integer overflow when reading RLE compressed TGA files

Also switched buffers to dynamic byte arrays and fixed some comments.

Author: galfar

Source/ImagingTarga.pas

@@ -54,7 +54,7 @@ implementation
   STargaSignature = 'TRUEVISION-XFILE';
 
 type
-  { Targa file header.}
+  { Targa file header }
   TTargaHeader = packed record
     IDLength: Byte;
     ColorMapType: Byte;
@@ -70,7 +70,7 @@ implementation
     Desc: Byte;
   end;
 
-  { Footer at the end of TGA file.}
+  { Footer at the end of TGA file }
   TTargaFooter = packed record
     ExtOff: UInt32;               // Extension Area Offset
     DevDirOff: UInt32;            // Developer Directory Offset
@@ -108,34 +108,40 @@ function TTargaFileFormat.LoadData(Handle: TImagingHandle;
 
   procedure LoadRLE;
   var
-    I, CPixel, Cnt: LongInt;
+    I: Integer;
+    CurrentPixel, CountPixels: NativeInt;
     Bpp, Rle: Byte;
-    Buffer, Dest, Src: PByte;
-    BufSize: LongInt;
+    Dest, Src: PByte;
+    Buffer: TDynByteArray;
+    BufSize: NativeInt;
+    BytesConsumedFromBuffer, SeekOffset: NativeInt;
   begin
     with GetIO, Images[0] do
     begin
       // Allocates buffer large enough to hold the worst case
       // RLE compressed data and reads then from input
       BufSize := Width * Height * FmtInfo.BytesPerPixel;
       BufSize := BufSize + BufSize div 2 + 1;
-      GetMem(Buffer, BufSize);
-      Src := Buffer;
-      Dest := Bits;
+
+      SetLength(Buffer, BufSize);
       BufSize := Read(Handle, Buffer, BufSize);
 
-      Cnt := Width * Height;
+      Src := @Buffer[0];
+      Dest := Bits;
+
+      CountPixels := Width * Height;
       Bpp := FmtInfo.BytesPerPixel;
-      CPixel := 0;
-      while CPixel < Cnt do
+      CurrentPixel := 0;
+
+      while CurrentPixel < CountPixels do
       begin
         Rle := Src^;
         Inc(Src);
         if Rle < 128 then
         begin
           // Process uncompressed pixel
           Rle := Rle + 1;
-          CPixel := CPixel + Rle;
+          CurrentPixel := CurrentPixel + Rle;
           for I := 0 to Rle - 1 do
           begin
             // Copy pixel from src to dest
@@ -153,7 +159,7 @@ function TTargaFileFormat.LoadData(Handle: TImagingHandle;
         begin
           // Process compressed pixels
           Rle := Rle - 127;
-          CPixel := CPixel + Rle;
+          CurrentPixel := CurrentPixel + Rle;
           // Copy one pixel from src to dest (many times there)
           for I := 0 to Rle - 1 do
           begin
@@ -168,10 +174,13 @@ function TTargaFileFormat.LoadData(Handle: TImagingHandle;
           Inc(Src, Bpp);
         end;
       end;
-      // set position in source to real end of compressed data
-      Seek(Handle, -(BufSize - (PtrUInt(Src) - PtrUInt(Buffer))),
-        smFromCurrent);
-      FreeMem(Buffer);
+
+      // Set position in source to the real end of compressed data
+      BytesConsumedFromBuffer := PtrUInt(Src) - PtrUInt(Buffer);
+      SeekOffset := BytesConsumedFromBuffer - BufSize;
+      Assert(SeekOffset <= 0);
+
+      Seek(Handle, SeekOffset, smFromCurrent);
     end;
   end;
 
@@ -196,7 +205,7 @@ function TTargaFileFormat.LoadData(Handle: TImagingHandle;
       3, 11: Format := ifGray8;
     end;
     // Format was not assigned by previous testing (it should be in
-    // well formed targas), so formats which reflects bit dept are selected
+    // well formed Targas), so format which reflect the bit depth is selected
     if Format = ifUnknown then
       case Hdr.PixelSize of
         8: Format := ifGray8;
@@ -268,8 +277,8 @@ function TTargaFileFormat.LoadData(Handle: TImagingHandle;
         Format := ifX1R5G5B5;
     end;
 
-    // We must find true end of file and set input' position to it
-    // paint programs appends extra info at the end of Targas
+    // We must find true end of file and set input's position to it.
+    // Some paint programs appends extra info at the end of Targas,
     // some of them multiple times (PSP Pro 8)
     repeat
       ExtFound := False;
@@ -296,7 +305,7 @@ function TTargaFileFormat.LoadData(Handle: TImagingHandle;
       end;
     until (not ExtFound) and (not FooterFound);
 
-    // Some editors save targas flipped
+    // Some editors save Targas flipped
     if Hdr.Desc < 31 then
       FlipImage(Images[0]);
 
@@ -316,7 +325,7 @@ function TTargaFileFormat.SaveData(Handle: TImagingHandle;
 
   procedure SaveRLE;
   var
-    Dest: PByte;
+    DestBuffer: TDynByteArray;
     WidthBytes, Written, I, Total, DestSize: LongInt;
 
     function CountDiff(Data: PByte; Bpp, PixelCount: Longint): LongInt;
@@ -407,10 +416,12 @@ function TTargaFileFormat.SaveData(Handle: TImagingHandle;
       begin
         DiffCount := CountDiff(Data, Bpp, PixelCount);
         SameCount := CountSame(Data, Bpp, PixelCount);
+
         if (DiffCount > MaxRun) then
           DiffCount := MaxRun;
         if (SameCount > MaxRun) then
           SameCount := MaxRun;
+
         if (DiffCount > 0) then
         begin
           Dest^ := Byte(DiffCount - 1);
@@ -449,19 +460,16 @@ function TTargaFileFormat.SaveData(Handle: TImagingHandle;
       WidthBytes := Width * FmtInfo.BytesPerPixel;
       DestSize := WidthBytes * Height;
       DestSize := DestSize + DestSize div 2 + 1;
-      GetMem(Dest, DestSize);
+      SetLength(DestBuffer, DestSize);
       Total := 0;
-      try
-        for I := 0 to Height - 1 do
-        begin
-          RleCompressLine(@PByteArray(Bits)[I * WidthBytes], Width,
-            FmtInfo.BytesPerPixel, @PByteArray(Dest)[Total], Written);
-          Total := Total + Written;
-        end;
-        GetIO.Write(Handle, Dest, Total);
-      finally
-        FreeMem(Dest);
+
+      for I := 0 to Height - 1 do
+      begin
+        RleCompressLine(@PByteArray(Bits)[I * WidthBytes], Width,
+          FmtInfo.BytesPerPixel, @DestBuffer[Total], Written);
+        Total := Total + Written;
       end;
+      GetIO.Write(Handle, DestBuffer, Total);
     end;
   end;
 
@@ -493,11 +501,10 @@ function TTargaFileFormat.SaveData(Handle: TImagingHandle;
     // Choose image type
     if FmtInfo.IsIndexed then
       Hdr.ImageType := Iff(FUseRLE, 9, 1)
+    else if FmtInfo.HasGrayChannel then
+      Hdr.ImageType := Iff(FUseRLE, 11, 3)
     else
-      if FmtInfo.HasGrayChannel then
-        Hdr.ImageType := Iff(FUseRLE, 11, 3)
-      else
-        Hdr.ImageType := Iff(FUseRLE, 10, 2);
+      Hdr.ImageType := Iff(FUseRLE, 10, 2);
 
     Write(Handle, @Hdr, SizeOf(Hdr));
 
@@ -520,7 +527,7 @@ function TTargaFileFormat.SaveData(Handle: TImagingHandle;
     end;
 
     if FUseRLE then
-      // Save rle compressed mode images
+      // Save RLE compressed mode images
       SaveRLE
     else
       // Save uncompressed mode images
@@ -580,8 +587,7 @@ initialization
 {
   File Notes:
 
- -- TODOS ----------------------------------------------------
-    - nothing now
+  * More recent changes are in VCS history *
 
   -- 0.21 Changes/Bug Fixes -----------------------------------
     - MakeCompatible method moved to base class, put ConvertToSupported here.