Issue 382 in webp: Integer Overflow on loop count > UINT16 in gif2webp

[dani… via monorail]

Status: New
Owner: ----
Labels: Type-Defect Priority-Medium

New issue 382 by dani...@tumblr.com: Integer Overflow on loop count > UINT16 in gif2webp
https://bugs.chromium.org/p/webp/issues/detail?id=382

What steps will reproduce the problem?
1. Using libwebp master
2. Build the gif2webp example tool
3. Download the example gif attached
4. Run `gif2webp -mt -lossy test_gif.gif -o test_out.webp

Observe the "ERROR (WEBP_MUX_INVALID_ARGUMENT): Could not update loop count."

What is the expected output? What do you see instead?

- Expected output is the gif getting converted, instead it just fails.


What version of the product are you using? On what operating system?

- libwebp1.0.0 using macOS Sierra v 10.12.6


Please provide any additional information below.

- Possible fix is to check that "loop_count < UINT16_MAX" on this line

https://chromium.googlesource.com/webm/libwebp/+/master/examples/gif2webp.c#481

In order to prevent the overflow when doing loop_count += 1

Attachments:
test_gif.gif 864 KB

--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

[jz… via monorail]

Updates:
Status: Accepted

Comment #1 on issue 382 by jz...@google.com: Integer Overflow on loop count > UINT16 in gif2webp
https://bugs.chromium.org/p/webp/issues/detail?id=382#c1

Thanks for the report. -loop_compatibilty could be used in this case to work around the bug. That or the file recreated without a loop count (= infinite).

The origin of the +1 is crbug.com/649264. We adjust the gif loop count to match the behavior in chrome where with webp the count is decreased by 1. When we cap at 65535 we'd technically play back one less time than the source, but I don't think that's a major issue.

[bugdro… via monorail]

Comment #2 on issue 382 by bugd...@chromium.org: Integer Overflow on loop count > UINT16 in gif2webp
https://bugs.chromium.org/p/webp/issues/detail?id=382#c2

The following revision refers to this bug:
https://chromium.googlesource.com/webm/libwebp/+/af0e4fbb0669026659983691a15dad23a9f196c1

commit af0e4fbb0669026659983691a15dad23a9f196c1
Author: James Zern <jz...@google.com>
Date: Fri May 11 19:10:25 2018

gif2webp: fix transcode of loop count=65535

with loop_compatibility disabled (the default), non-zero loop counts
will be incremented by 1 for browser rendering compatibility. the max,
65535, is a special case as the muxer will fail if it is exceeded; avoid
increasing the limit in this case. this isn't 100% correct, but should
be close enough given the high number of iterations.

BUG=webp:382

Change-Id: Icde3e98a58e9ee89604a72fafda30ab71060dec5

[modify] https://crrev.com/af0e4fbb0669026659983691a15dad23a9f196c1/examples/anim_util.h
[modify] https://crrev.com/af0e4fbb0669026659983691a15dad23a9f196c1/examples/gif2webp.c
[modify] https://crrev.com/af0e4fbb0669026659983691a15dad23a9f196c1/examples/anim_util.c
[modify] https://crrev.com/af0e4fbb0669026659983691a15dad23a9f196c1/examples/anim_diff.c

[bugdro… via monorail]

Updates:
Labels: merge-merged-1.0.0

Comment #3 on issue 382 by bugd...@chromium.org: Integer Overflow on loop count > UINT16 in gif2webp
https://bugs.chromium.org/p/webp/issues/detail?id=382#c3

The following revision refers to this bug:

https://chromium.googlesource.com/webm/libwebp/+/4b282e13ad0cf7f1a3b4431788bddef1d6b11762

commit 4b282e13ad0cf7f1a3b4431788bddef1d6b11762
Author: James Zern <jz...@google.com>
Date: Fri May 11 21:31:02 2018

gif2webp: fix transcode of loop count=65535

with loop_compatibility disabled (the default), non-zero loop counts
will be incremented by 1 for browser rendering compatibility. the max,
65535, is a special case as the muxer will fail if it is exceeded; avoid
increasing the limit in this case. this isn't 100% correct, but should
be close enough given the high number of iterations.

BUG=webp:382

Change-Id: Icde3e98a58e9ee89604a72fafda30ab71060dec5

(cherry picked from commit af0e4fbb0669026659983691a15dad23a9f196c1)

[modify] https://crrev.com/4b282e13ad0cf7f1a3b4431788bddef1d6b11762/examples/anim_util.h
[modify] https://crrev.com/4b282e13ad0cf7f1a3b4431788bddef1d6b11762/examples/gif2webp.c
[modify] https://crrev.com/4b282e13ad0cf7f1a3b4431788bddef1d6b11762/examples/anim_util.c
[modify] https://crrev.com/4b282e13ad0cf7f1a3b4431788bddef1d6b11762/examples/anim_diff.c

[jz… via monorail]

Updates:
Labels: v1.0.1
Status: Fixed

Comment #4 on issue 382 by jz...@google.com: Integer Overflow on loop count > UINT16 in gif2webp
https://bugs.chromium.org/p/webp/issues/detail?id=382#c4

Thanks again for the report. This is fixed in the master branch and I cherry-picked it to 1.0.0 to allow downstream projects to pull it into their distribution. This will be included in the 1.0.1 release, but we don't have plans for that currently.

[dani… via monorail]

Comment #5 on issue 382 by dani...@tumblr.com: Integer Overflow on loop count > UINT16 in gif2webp
https://bugs.chromium.org/p/webp/issues/detail?id=382#c5

Awesome news!