oss fuzz issue 62136 webp ASSERT: dec->last_row_ < last_row
Categories
(Core :: Graphics: ImageLib, defect, P1)
Tracking
()
People
(Reporter: tnikkel, Assigned: tnikkel)
References
Details
(Whiteboard: [adv-main118-][adv-esr115.3-])
Attachments
(2 files)
127 bytes,
application/octet-stream
|
Details | |
48 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr102-
RyanVM
:
approval-mozilla-esr115+
tjr
:
sec-approval+
|
Details | Review |
Filing this to track https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62136
Assignee | ||
Comment 1•1 year ago
|
||
This is the patch that is supposed to fix it
https://chromium.googlesource.com/webm/libwebp/+/95ea5226c870449522240ccff26f0b006037c520
Comment 2•1 year ago
|
||
We'll need a patch ready for uplift before the RC build next Monday to make this into 118. Can you do this, Timothy?
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Assignee | ||
Comment 3•1 year ago
|
||
When loading the testcase locally the assert does not fail for me (the condition of the assert passes even if I check it manually by adding code). Not sure why. This probably means we can't verify a fix.
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Comment 4•1 year ago
|
||
Assignee | ||
Comment 5•1 year ago
|
||
Comment on attachment 9352821 [details]
Bug 1852749. Cherry-pick upstream libwebp fix. r?#gfx-reviewers
Beta/Release Uplift Approval Request
- User impact if declined: sec-high
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky):
- String changes made/needed:
- Is Android affected?: Yes
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-high
- User impact if declined: sec-high
- Fix Landed on Version: 119
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): fix for sec issue
Comment 6•1 year ago
|
||
Tyson, just yesterday it looked like we want to land this ASAP as it was created as a follow-up to the other webp bug.
But with Timothy saying he can't reproduce in comment 3, I would like someone to help verify. Maybe you can be of help here?
Assignee | ||
Comment 7•1 year ago
|
||
Comment on attachment 9352821 [details]
Bug 1852749. Cherry-pick upstream libwebp fix. r?#gfx-reviewers
Security Approval Request
- How easily could an exploit be constructed based on the patch?: not sure, this patch is already public in the libwebp repo
- Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Unknown
- Which older supported branches are affected by this flaw?: all
- If not all supported branches, which bug introduced the flaw?: None
- Do you have backports for the affected branches?: No
- If not, how different, hard to create, and risky will they be?: probably not hard, not sure how much this file has changed in libwebp, probably not much/at all. risk should be the same as this patch
- How likely is this patch to cause regressions; how much testing does it need?: unlikely
- Is Android affected?: Yes
Comment 8•1 year ago
|
||
I am unable to reproduce this issue with the provided test case on m-c 20230911-d2fd68fd89bb (debug). This build should contain the issue. The image loads I see a blue rectangle.
tnikkel I assume the assertions from libwebp should be triggerable in a debug build, is this true?
Assignee | ||
Comment 9•1 year ago
|
||
I don't know, I wrote my own code at the assertion site (printfs) to both verify that my code was being hit and to see if the assert condition failed. My code was hit but the assert condition did not fail.
The chromium issue was marked verified by cluster fuzz after the patch landed.
Assignee | ||
Comment 10•1 year ago
|
||
FYI, the chromium issue has been opened to the public. The testcase still seems to be private though.
Assignee | ||
Comment 11•1 year ago
|
||
Also, I added an assert using the same function to libwebp in my firefox tree that I knew would fail to see what would happen, and it crashed the tab, so that is good. That most likely means our fuzzing builds would also crash when failing an assert in libwebp.
Comment 12•1 year ago
|
||
Should I expect an uplift request coming today for our last beta or one for the RC on Monday?
Assignee | ||
Comment 13•1 year ago
|
||
(In reply to Pascal Chevrel:pascalc from comment #12)
Should I expect an uplift request coming today for our last beta or one for the RC on Monday?
I'm just waiting on sec-approval. I'll trigger landing (or someone else can) as soon as it's granted. Both the patch and the original bug are public now, so there doesn't seem to be any reason to wait.
The uplift request is already there.
Comment 14•1 year ago
|
||
Comment on attachment 9352821 [details]
Bug 1852749. Cherry-pick upstream libwebp fix. r?#gfx-reviewers
approved to land and uplift
Comment 15•1 year ago
|
||
Pushed by tnikkel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3c159cd917bb Cherry-pick upstream libwebp fix. r=gfx-reviewers,lsalzman
![]() |
||
Comment 16•1 year ago
|
||
Comment 17•1 year ago
|
||
Comment on attachment 9352821 [details]
Bug 1852749. Cherry-pick upstream libwebp fix. r?#gfx-reviewers
Approved for landing on mozilla-beta before the merge, will be in the 118.0 release candidate, thanks.
Updated•1 year ago
|
Comment 19•1 year ago
|
||
Comment on attachment 9352821 [details]
Bug 1852749. Cherry-pick upstream libwebp fix. r?#gfx-reviewers
Approved for 115.3esr. ESR102 will be EOL at the end of this current cycle, so no need to uplift there.
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Comment 21•1 year ago
|
||
Upstream has indicated that this is not a security bug, so I am clearing the sec flag to put it in the triage queue to get a double check before making it public. Also excluding it from advisories.
Updated•1 year ago
|
Comment 22•1 year ago
|
||
Sounds okay to me. From the libwebp bug:
This was an incorrect check in an assert(). A release build would not be negatively affected.
Description
•