View Issue Details

IDProjectCategoryView StatusLast Update
0034185FPCLCLpublic2018-08-28 10:50
ReporterSergey Tkachenko Assigned ToMarco van de Voort  
PrioritynormalSeverityminorReproducibilityhave not tried
Status resolvedResolutionfixed 
PlatformWin 32 and 64OSWindows 
Summary0034185: TIcon crash
DescriptionThe attached file crashes TIcon on loading
Steps To ReproduceUse Icon.LoadFromFile

or

Try to assign this icon as a project icon in "Project | Project Options...".
TagsNo tags attached.
Fixed in Revision39676
FPCOldBugId
FPCTarget
Attached Files

Activities

Sergey Tkachenko

2018-08-27 12:53

reporter  

ActionTestUni_Icon.ico (164,126 bytes)   
ActionTestUni_Icon.ico (164,126 bytes)   

wp

2018-08-27 21:34

reporter   ~0110331

Last edited: 2018-08-27 21:35

View 3 revisions

Could somebody move this issue to FPC because it is an FCL issue:

The crash happens because this icon contains several png images and is related to the usage of palettes. The faulty routine is TFPReaderPNG.InternalRead where the crash occurs in "FPalette.Free":

  procedure TFPReaderPNG.InternalRead (Str:TStream; Img:TFPCustomImage);
  begin
    [...]
    try
      [...] // reading here...
    finally
      [...]
      if not img.UsePalette and assigned(FPalette) then
        begin
        FPalette.Free; // <--- CRASH HERE
        end;
    end;
  end;

Greenfish Icon Editor shows that the first png image, at index 4 in the ico file, has 256 colors and thus uses a palette which is read by the reader into its own palette (FPalette). After having read the png image, in the finally section of the code above, the reader checks wheter this palette is <> nil and destroys it. But the pointer to it is still <> nil.

The same reader is reused when the next png compressed image is read, at index 8. It is a 32-bit color image without a palette. Therefore the reader does not create a new palette for it, but still has the dangling pointer in FPalette. Therefore the check for not being nil is passed and the reader tries to free FPalette again - of course, this fails because the palette has not been allocated.

Therefore, the solution is simple: Instead of "FPalette.Free" call "FreeAndNil(FPalette)".

wp

2018-08-27 22:46

reporter  

fpreadpng.pp.patch (383 bytes)   
Index: fcl-image/src/fpreadpng.pp
===================================================================
--- fcl-image/src/fpreadpng.pp	(revision 39657)
+++ fcl-image/src/fpreadpng.pp	(working copy)
@@ -833,7 +833,7 @@
     ZData.Free;
     if not img.UsePalette and assigned(FPalette) then
       begin
-      FPalette.Free;
+      FreeAndNil(FPalette);
       end;
   end;
 end;
fpreadpng.pp.patch (383 bytes)   

wp

2018-08-27 22:47

reporter   ~0110333

Added a patch.

Marco van de Voort

2018-08-28 10:50

manager   ~0110338

Thanks, done

Issue History

Date Modified Username Field Change
2018-08-27 12:53 Sergey Tkachenko New Issue
2018-08-27 12:53 Sergey Tkachenko File Added: ActionTestUni_Icon.ico
2018-08-27 19:27 Bart Broersma LazTarget => -
2018-08-27 19:27 Bart Broersma Status new => confirmed
2018-08-27 21:34 wp Note Added: 0110331
2018-08-27 21:35 wp Note Edited: 0110331 View Revisions
2018-08-27 21:35 wp Note Edited: 0110331 View Revisions
2018-08-27 21:57 Bart Broersma Project Lazarus => FPC
2018-08-27 22:46 wp File Added: fpreadpng.pp.patch
2018-08-27 22:47 wp Note Added: 0110333
2018-08-28 10:50 Marco van de Voort Fixed in Revision => 39676
2018-08-28 10:50 Marco van de Voort Note Added: 0110338
2018-08-28 10:50 Marco van de Voort Status confirmed => resolved
2018-08-28 10:50 Marco van de Voort Resolution open => fixed
2018-08-28 10:50 Marco van de Voort Assigned To => Marco van de Voort