Issue 518 in webp: IncreasePreviousDuration may case encoded_frames_ array overflow

13 views
Skip to first unread message

tomwe… via monorail

unread,
Apr 22, 2021, 10:14:24 AMApr 22
to webp-d...@webmproject.org
Status: New
Owner: ----
Labels: Type-Defect Priority-Medium

New issue 518 by tomwe...@gmail.com: IncreasePreviousDuration may case encoded_frames_ array overflow
https://bugs.chromium.org/p/webp/issues/detail?id=518

What steps will reproduce the problem?

1. build webp with debug mode with get more obvious error.
2. use gif2webp convert attach file 'test.gif' to webp use this command: ./gif2webp -kmin 0 -kmax 0 -lossy -q 75 test.gif -o output.webp

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

gif2webp will abort by assert fail

gif2webp: /home/tomwei7/workspace/libwebp/src/mux/anim_encode.c:343: GetFrame: Assertion `enc->start_ + position < enc->size_' failed.


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

Latest,ArchLinux

Please provide any additional information below.

I do some debug on this issues, that because if kmax and kmin is 0 that encoded_frames_ array will set to minimum size 2. IncreasePreviousDuration will add transparent frame to avoid duration overflow this can lead to encoded_frames_ array full and will overflow on next step.

Attachments:
test.gif 1007 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

tomwe… via monorail

unread,
Apr 22, 2021, 12:00:55 PMApr 22
to webp-d...@webmproject.org

Comment #1 on issue 518 by tomwe...@gmail.com: IncreasePreviousDuration may case encoded_frames_ array overflow
https://bugs.chromium.org/p/webp/issues/detail?id=518#c1

I has fix this issues by check encoded_frames_ count and FlushFrames if necessary after IncreasePreviousDuration.

Attachments:
0001-Fix-encoded_frames_-array-overflow.patch 1.5 KB

jz… via monorail

unread,
Apr 30, 2021, 1:01:32 AMApr 30
to webp-d...@webmproject.org
Updates:
Owner: jz...@google.com
Status: Accepted

Comment #2 on issue 518 by jz...@google.com: IncreasePreviousDuration may case encoded_frames_ array overflow
https://bugs.chromium.org/p/webp/issues/detail?id=518#c2

Thanks for the report. I can reproduce this at v1.2.0-45-g05b72d42. I'll take a closer look.

jz… via monorail

unread,
May 20, 2021, 4:07:23 PMMay 20
to webp-d...@webmproject.org
Updates:
Cc: urv...@google.com

Comment #4 on issue 518 by jz...@google.com: IncreasePreviousDuration may case encoded_frames_ array overflow
https://bugs.chromium.org/p/webp/issues/detail?id=518#c4

(No comment was entered for this change.)

Git Watcher via monorail

unread,
Jun 23, 2021, 9:53:06 PMJun 23
to webp-d...@webmproject.org

Comment #5 on issue 518 by Git Watcher: IncreasePreviousDuration may case encoded_frames_ array overflow
https://bugs.chromium.org/p/webp/issues/detail?id=518#c5

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

commit 116d235c30c0592aef4678dde3e7dbac1449c6e6
Author: James Zern <jz...@google.com>
Date: Tue Jun 22 23:07:04 2021

anim_encode: Fix encoded_frames_[] overflow

Check encoded_frames_ count and call FlushFrames if necessary after
IncreasePreviousDuration. Avoids an overflow in encoded_frames_[] with
-kmax 0 and an assertion failure related to the previous and keyframe
durations when a frame is forced in this way.

Based on patch by tomwei7g <at> gmail

Bug: webp:518
Change-Id: Idef685e6c06a67d48fcdc048265ca0e672a01263

[modify] https://crrev.com/116d235c30c0592aef4678dde3e7dbac1449c6e6/src/mux/anim_encode.c

jz… via monorail

unread,
Jun 23, 2021, 9:56:23 PMJun 23
to webp-d...@webmproject.org
Updates:
Status: Fixed

Comment #6 on issue 518 by jz...@google.com: IncreasePreviousDuration may case encoded_frames_ array overflow
https://bugs.chromium.org/p/webp/issues/detail?id=518#c6

This should be fixed now. Thanks again for the report and the basis for the fix.
Reply all
Reply to author
Forward
0 new messages