loop in Image::ExifTool::FlashPix

[Archive]

[Originally posted by humyo on 2007-07-16 10:46:18-07]

Hi,

I've hit a case where Image::ExifTool::FlashPix::LoadChain loops on a file. It seems that MS Office files are often (always?) recognised as FlashPix, I assume that they are both based on a FAT-filesystem like structure.

In this case, the $sect value was repeatedly zero.

The following patch attempts to detect loops in the sector numbers (which would give an infinite loop) and also sets an arbitrary upper bound on the chain length - which may be completely unreasonable, I'm not sure.

Could this, or something like this, perhaps go into a future release of the module?

Code:

--- FlashPix.pm 2007-07-16 10:57:58.000000000 +0100
+++ FlashPix.pm.new     2007-07-16 10:55:43.000000000 +0100
@@ -891,8 +891,17 @@
     my ($raf, $sect, $fatPt, $sectSize, $hdrSize) = @_;
     return undef unless $raf;
     my $chain = '';
+
+    # Avoid loops in the chain
+    my %seen_sect = ();
+    # Don't know what a sane upper bound on the number of sectors is
+    my $okToLoop = 4096;
+
     for (;;) {
         last if $sect == END_OF_CHAIN;
+        return if $seen_sect{$sect};
+        return unless $okToLoop--;
+
         my $offset = $sect * $sectSize + $hdrSize;
         my $buff;
         unless ($raf->Seek($offset, 0) and $raf->Read($buff, $sectSize) == $sectSize) {
@@ -901,6 +910,7 @@
         $chain .= $buff;
         # step to next sector in chain
         return undef if $sect * 4 > length($$fatPt) - 4;
+        $seen_sect{$sect}++;
         $sect = Get32u($fatPt, $sect * 4);
     }
     return $chain;

[Archive]

[Originally posted by exiftool on 2007-07-16 11:50:36-07]

Wow, another hang bug discovered.  You're good.

Thanks for pointing this out!

Yes, Microsoft office files are the same format as flashpix.

I have added the infinite recursion test, but not the maximum
chain length test.  Is this necessary?  Do you have some files with
unusually long chains?

Anyway, this update has been included in the
https://exiftool.org/Image-ExifTool-6.94.tar.gz" target="_blank">6.94
pre-release.

- Phil

[Archive]

[Originally posted by humyo on 2007-07-16 12:29:31-07]

We've got a lot of sample data, so I suspect we may hit a lot of issues. Thanks very much for being responsive to them, and for including a fix in the latest update.

The upper limit on the number of loops was just paranoia.

And, here's another. We've got an ASF file which normally loops, but when run under '-v -v -v', it dies with:

Code:

 Data
      0000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [................]
      0010: 97 01 00 00 00 00 00 00 01 01 82 00 00 01 5d 00 [..............].]
      0020: 00 00 00 00 00 81 81 00 00 00 00 00 08 f5 20 00 [.............. .]
      0030: 00 01 00 00 00 e3 07 00 00 01 b6 10 60 62 30 0b [............`b0.]
      0040: fb 6f 66 2f 7a 82 f9 84 6c cc 38 1b cd 37 4f 60 [.of/z...l.8..7O`]
      [snip 833482 bytes]
  00000000-0000-0000-0000-000000000000
  Warning = Truncated file

It turns out that $size is being set to '-24' in the ProcessASF function, presumably because of zeros being passed to Get64u. This patch detects and warns in such a case (and stops the inf. loop in the non-verbose case):

Code:

--- ASF.pm.orig 2007-07-16 13:01:40.000000000 +0100
+++ ASF.pm      2007-07-16 13:25:52.000000000 +0100
@@ -862,6 +862,10 @@
             $rtnVal = 1;
         }
         my $size = Image::ExifTool::Get64u(\$buff, 16) - 24;
+        if ($size < 0) {
+            $err = 'Undersized ASF object';
+            last;
+        }
         if ($size > 0xffffffff) {
             $err = 'Large ASF objects not supported';
             last;

[Archive]

[Originally posted by humyo on 2007-07-16 13:25:48-07]

Ah...sorry, I can see that this ASF fix (check for -ve size) is in your 6.94 pre-release. Please ignore the previous (and thanks very much for the fix).