Bug 23684 – Endless loop for image with zero duration frame

• Component: Images
• Status: RESOLVED CONFIGURATION CHANGED

[Lyon Chen]

In function BitmapImage::startAnimation(), there's a potential endless loop when the duration of frames is zero, which can be observed in aircanada.com for its searching page.

Following is the verified fix:

diff --git a/WebCore/platform/graphics/BitmapImage.cpp b/WebCore/platform/graphics/BitmapImage.cpp
index 45b32ab..e3ef8b0 100644
--- a/WebCore/platform/graphics/BitmapImage.cpp
+++ b/WebCore/platform/graphics/BitmapImage.cpp
@@ -323,6 +323,10 @@ void BitmapImage::startAnimation(bool catchUpIfNecessary)
             if (time < frameAfterNextStartTime)
                 break;
 
+            /* Is the frame duration is zero, which means we should not animate! */
+            if (frameAfterNextStartTime == m_desiredFrameStartTime)
+                break;
+
             // Yes; skip over it without notifying our observers.
             if (!internalAdvanceAnimation(true))
                 return;

[Mark Rowe (bdash)]

<rdar://problem/6548504>

Can you attach an image that causes the endless looping?

[Lyon Chen]

I got into this situation due to another bug in my own code. I think just in case such a situation occurs it's better to have a way out. But I don't know any such animated GIF image (with zero duration frames) out in the wild.

Peter, can this occur or does a "0-duration frame" always indicate a programmer error or a bug in the image decoder (in which case there should be an ASSERT)?

[Adam Treat]

I think it is important to note that a malicious gif could be constructed to have this feature though.

(In reply to comment #5)
> I think it is important to note that a malicious gif could be constructed to
> have this feature though.

Please construct one for the regression test.

[Peter Kasting]

I'd need to see a sample GIF where this happens to know what the effects are.  You can probably hack an existing one and just force 0 into some frames' duration slots.

The reason I say this is because there's already a bunch of code elsewhere that enforces minimum frame durations, and I don't know whether that's already run by the time we get here.  So it's not clear to me whether this can really be a problem.  Plus, I thought I tested some real ad GIFs with 0 frame durations and didn't have problems?  But my memory could be bad.

[Jeremy Zerfas]

This ticket has been open for several years and I just thought I would point out that although this bug seems to have been valid back then, some of the relevant code has been refactored since then and there is no longer a potential for an endless loop. In the current code, if the containing loop doesn't exit the function, then on each iteration of that loop it instead increments frameAfterNextStartTime by the frameDurationAtIndex() function (which normally never returns a value less than 0.011 seconds) and eventually causes the loop to end.

I'm attaching two GIFs I created that contain zero duration frames in case anyone wants them for testing. Both GIFs contain a repeating animation that shows the numbers 1, 2, and 3 after each other. In "Animated GIF with Single Zero Duration Frame.gif", the first and third frames have a one second delay but the second frame has a zero second delay. In "Animated GIF with All Zero Duration Frames.gif" all the frames have zero second delays.

It looks like zero duration frames are allowed by GIFs and do have some valid uses, see http://www.imagemagick.org/Usage/anim_basics/#zero . Mozilla added some code back in late 2013 to respect those zero duration frames for non-looping GIFs but it looks like they just recently found out that the new code is causing problems with some (poorly made) GIFs on the web and will be removing that code, see https://bugzilla.mozilla.org/show_bug.cgi?id=890743 .

[Jeremy Zerfas]

Created attachment 258042 [details]
Repeating GIF animation where first and third frames have a one second delay but second frame has a zero second delay.

[Jeremy Zerfas]

Created attachment 258043 [details]
Repeating GIF animation where all frames have a zero second delay.

Marking as resolved based on the observation that an infinite loop can no longer occur here.