SEGV on libwebp WebPSafeFree -> malloc_decls.h free
Categories
(Core :: Graphics: ImageLib, defect)
Tracking
()
People
(Reporter: sourc7, Assigned: RyanVM)
References
(Regression)
Details
(6 keywords, Whiteboard: [reporter-external] [client-bounty-form] [verif?][post-critsmash-triage][adv-main101+])
Attachments
(11 files, 2 obsolete files)
4.31 KB,
text/plain
|
Details | |
4.09 KB,
text/plain
|
Details | |
380 bytes,
text/html
|
Details | |
48 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
|
Details | Review |
201 bytes,
text/plain
|
Details | |
4.30 KB,
text/plain
|
Details | |
918 bytes,
text/html
|
Details | |
1011 bytes,
text/html
|
Details | |
1.05 KB,
text/html
|
Details | |
1.05 KB,
text/html
|
Details | |
4.30 KB,
text/plain
|
Details |
After run canvas.toBlob(callback, "image/webp")
and canvas.toDataURL()
in loop, then when Firefox trigger free to avoid OOM, surprisingly Firefox crashed with SIGSEGV WebPSafeFree
to malloc_decls.h free
crash signature.
Tested on:
- Firefox 98.0.2 (32-bit) on Arch Linux
- Firefox Nightly 100.0a1 (2022-03-24) (32-bit) on Arch Linux
Steps to reproduce:
- Open Firefox (32-bit)
- Visit attached testcase.html
- After few seconds Firefox crashed with "free" crash signature
(If Firefox crashed withmozalloc_abort
signature, try again until it crashed withfree
crash signature)
Crash report minidump:
eip = 0x5661a62f esp = 0xe4457f80 ebp = 0xe4458008 ebx = 0x56675000
esi = 0x00001010 edi = 0x10100000 eax = 0x00000001 ecx = 0x565b9500
edx = 0x00000000 efl = 0x00010206
OS|Linux|0.0.0 Linux 5.16.13-arch1-1 #1 SMP PREEMPT Tue, 08 Mar 2022 20:07:36 +0000 x86_64
CPU|x86|AuthenticAMD family 23 model 96 stepping 1|12
GPU|||
Crash|SIGSEGV / SEGV_MAPERR|0x10100000|6
6|0|firefox-bin|free|hg:hg.mozilla.org/mozilla-central:memory/build/malloc_decls.h:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|54|0x6f
6|1|libxul.so|WebPSafeFree|hg:hg.mozilla.org/mozilla-central:media/libwebp/src/utils/utils.c:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|219|0x8
6|2|libxul.so|VP8LBitWriterWipeOut|hg:hg.mozilla.org/mozilla-central:media/libwebp/src/utils/bit_writer_utils.c:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|257|0xb
6|3|libxul.so|VP8LEncodeStream|hg:hg.mozilla.org/mozilla-central:media/libwebp/src/enc/vp8l_enc.c:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|2033|0xf
6|4|libxul.so|EncodeAlphaInternal|hg:hg.mozilla.org/mozilla-central:media/libwebp/src/enc/alpha_enc.c:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|139|0x1f
6|5|libxul.so|CompressAlphaJob|hg:hg.mozilla.org/mozilla-central:media/libwebp/src/enc/alpha_enc.c:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|374|0x33
6|6|libxul.so|VP8EncStartAlpha|hg:hg.mozilla.org/mozilla-central:media/libwebp/src/enc/alpha_enc.c:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|413|0x10
6|7|libxul.so|WebPEncode|hg:hg.mozilla.org/mozilla-central:media/libwebp/src/enc/webp_enc.c:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|382|0x8
6|8|libxul.so|Encode|hg:hg.mozilla.org/mozilla-central:media/libwebp/src/enc/picture_enc.c:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|255|0x2a
6|9|libxul.so|WebPEncodeRGBA|hg:hg.mozilla.org/mozilla-central:media/libwebp/src/enc/picture_enc.c:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|273|0x38
6|10|libxul.so|nsWebPEncoder::InitFromData(unsigned char const*, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, nsTSubstring<char16_t> const&)|hg:hg.mozilla.org/mozilla-central:image/encoders/webp/nsWebPEncoder.cpp:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|138|0x2d
6|11|libxul.so|mozilla::dom::ImageEncoder::ExtractDataInternal(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, unsigned char*, int, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, bool, mozilla::layers::Image*, nsICanvasRenderingContextInternal*, mozilla::layers::CanvasRenderer*, nsIInputStream**, imgIEncoder*)|hg:hg.mozilla.org/mozilla-central:dom/base/ImageEncoder.cpp:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|414|0x19
6|12|libxul.so|mozilla::dom::EncodingRunnable::ProcessImageData(unsigned long long*, void**)|hg:hg.mozilla.org/mozilla-central:dom/base/ImageEncoder.cpp:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|163|0x44
6|13|libxul.so|mozilla::dom::EncodingRunnable::Run()|hg:hg.mozilla.org/mozilla-central:dom/base/ImageEncoder.cpp:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|186|0x8
6|14|libxul.so|nsThreadPool::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadPool.cpp:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|310|0x8
6|15|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|1167|0x8
6|16|libxul.so|mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|300|0x30
6|17|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|306|0xc
6|18|libxul.so|nsThread::ThreadFunc(void*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|389|0x8
6|19|libnspr4.so|_pt_root|hg:hg.mozilla.org/mozilla-central:nsprpub/pr/src/pthreads/ptthread.c:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|201|0x9
6|20|firefox-bin|set_alt_signal_stack_and_start(PthreadCreateParams*)|hg:hg.mozilla.org/mozilla-central:toolkit/crashreporter/pthread_create_interposer/pthread_create_interposer.cpp:a8d37f94a6364103b7f373b9938ce7a62d9db9e6|80|0x9
6|21|libc.so.6||||0x8a3de
6|22|libc.so.6||||0x122d1a
Reporter | ||
Comment 1•3 years ago
|
||
Reporter | ||
Comment 2•3 years ago
|
||
Reporter | ||
Comment 3•3 years ago
|
||
Here my Firefox 100.0a1 crash report link [@ free | WebPSafeFree] it shows SIGSEGV / SEGV_MAPERR on 0x10100000.
![]() |
||
Updated•3 years ago
|
Reporter | ||
Comment 4•3 years ago
|
||
From the crash report stack file and mozregression it's looks like it regression of Bug 1511670 - Implement WebP image encoder.
Updated•3 years ago
|
Updated•3 years ago
|
Comment 5•3 years ago
|
||
Set release status flags based on info from the regressing bug 1511670
Updated•3 years ago
|
Updated•3 years ago
|
Comment 6•3 years ago
|
||
I was not able to reproduce on 64-bit Firefox, but running 32-bit Firefox on 64-bit Linux worked. Possibly related, I start to see this warning repeated in the log caused by the page reloads up to and prior to the crash:
[GFX1-]: CreateDataSourceSurfaceWithStride failed to initialize Size(14131,2978), SurfaceFormat::B8G8R8A8, 56524, true
Comment 7•3 years ago
|
||
At this point, I am mostly convinced this is the OOM killer / Linux's overcommit at work due to running out of virtual memory, rather than a sec issue.
Comment 8•3 years ago
|
||
I believe the free issue is because of the ordering of these checks:
https://searchfox.org/mozilla-central/rev/766773ca7e3495a3f1ba365f47dffefdea29e3ee/media/libwebp/src/enc/vp8l_enc.c#1919
Normally we would have called VP8LBitWriterInit
but if the earlier calls failed, we don't initialize it at all and we try to free an uninitialized pointer off the stack here:
https://searchfox.org/mozilla-central/rev/766773ca7e3495a3f1ba365f47dffefdea29e3ee/media/libwebp/src/enc/vp8l_enc.c#2033
However to get to this point, we need to be memory starved to cause some of these allocations to fail. I haven't identified the memory leak yet. DMD isn't working (I filed bug 1762276 about that).
Comment 9•3 years ago
|
||
Unsure of the appropriate sec rating. Two issues here, the memory leak (or otherwise tied up), and the fallout from OOMs which I believe is in libwebp in this particular case.
Comment 10•3 years ago
|
||
sec-want
is for security enhancements/defense-in-depth kinds of bugs, never for potential vulnerabilities. You can leave a sec- keyword off and then we find it in triage. If you add one then it bypasses triage, so please only add a rating if you're sure.
Freeing an uninitialized value is theoretically exploitable: if you can get something else to leave a useful value in there you might end up prematurely freeing something else valuable and cause an exploitable UAF elsewhere. It's on the stack which puts a lot limits on how you might leave a useful value behind, but on the other hand if there's a way it would be more deterministic than trying to do that on the heap. Seems hard and unlikely to be exploited in practice, but definitely not impossible. sec-moderate
is appropriate
Comment 11•3 years ago
|
||
Thanks for the explanation Daniel, normally they already have a rating by the time they come to me :).
I've concluded there is no memory leak after finally getting DMD working. The testcase is able to put many of these encoding runnables in flight on background worker threads before they complete, and it gets worse over time. They do however free all their memory once they complete. We might be able to do better on our allocations on this code path, but overall the problem here is in libwebp. I will see about fixing this upstream.
Comment 12•3 years ago
•
|
||
Would you be willing to file this issue with Chrome/libwebp? I'm not sure if your test case will reproduce the exact issue, but you may quote the relevant bits from me in comment 8 to explain the issue. Feel free to cc me on that bug.
Reporter | ||
Comment 13•3 years ago
|
||
(In reply to Andrew Osmond [:aosmond] (he/him) from comment #12)
Would you be willing to file this issue with Chrome/libwebp? I'm not sure if your test case will reproduce the exact issue, but you may quote the relevant bits from me in comment 8 to explain the issue. Feel free to cc me on that bug.
Alright I've created new testcase it now more reliable and also works on Windows 11, moreover on Windows it able to crash with ACCESS_VIOLATION_WRITE
to changing address e.g. 0x10
, 0xb000010
, 0x80b1323
, 0xd144918
and more...
On Linux when combining the testcase with another content I also found the SIGSEGV
read sometimes able to change from 0x10100000
to 0xe2c00000
, 0xdda00000
and more..
I'll try file this as Security issue on webp chromium monorail, describe we encounter crash on Firefox at libwebp free, quote comment 8 and +cc on the issue, hopefully they will able to found the root cause.
Reporter | ||
Comment 14•3 years ago
|
||
(In reply to Andrew Osmond [:aosmond] (he/him) from comment #12)
Would you be willing to file this issue with Chrome/libwebp? I'm not sure if your test case will reproduce the exact issue, but you may quote the relevant bits from me in comment 8 to explain the issue. Feel free to cc me on that bug.
Alright I've recently filled security issue https://bugs.chromium.org/p/webp/issues/detail?id=565 on chromium webp monorail, I've also request +cc aosmond@mozilla.com to the issue, hopefully we able to figure it out.
Updated•3 years ago
|
Updated•3 years ago
|
Reporter | ||
Comment 15•3 years ago
|
||
Alright the WebP team has been fixed this upstream "and will be making its way to earlier release branches (through 0.6.1)" (quoting from James Zern WebP developer in libwebp issue tracker).
After applied commit https://github.com/webmproject/libwebp/commit/2de4b05a56e4da42d170f4af8b01327a3b4b251b to Firefox source build, I confirmed it no longer crash with "WebPSafeFree" signature.
Assignee | ||
Comment 16•3 years ago
|
||
Andrew, do we want to cherry-pick the upstream fix for 101?
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 17•3 years ago
|
||
Backport of upstream commit:
https://github.com/webmproject/libwebp/commit/2de4b05a56e4da42d170f4af8b01327a3b4b251b
Assignee | ||
Comment 18•3 years ago
|
||
![]() |
||
Comment 19•3 years ago
|
||
Fix WebP crash on OOM. r=aosmond
https://hg.mozilla.org/integration/autoland/rev/b1a6d489400499979f16cc982901f78ac504bc77
https://hg.mozilla.org/mozilla-central/rev/b1a6d4894004
Assignee | ||
Comment 20•3 years ago
|
||
Comment on attachment 9275561 [details]
Bug 1761275 - Fix WebP crash on OOM. r=aosmond
Beta/Release Uplift Approval Request
- User impact if declined: Publicly-disclosed WebP security vulnerability
- 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): Code is only used by the WebP encoder, which isn't widely used.
- String changes made/needed:
- Is Android affected?: Unknown
Assignee | ||
Comment 21•3 years ago
|
||
Comment on attachment 9275561 [details]
Bug 1761275 - Fix WebP crash on OOM. r=aosmond
Approved for 101.0b6.
Assignee | ||
Comment 22•3 years ago
|
||
uplift |
Updated•3 years ago
|
Comment 23•3 years ago
|
||
This is essentially just a DOS bug; annoying but not that dangerous.
Updated•3 years ago
|
Comment 24•3 years ago
|
||
I asked Google for a CVE.
Updated•3 years ago
|
Updated•3 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Comment 26•2 years ago
|
||
Raising the severity and the reward amount based on a new testcase Irvan submitted that shows more control over the crash and the possibility of creating an exploit.
Comment 27•2 years ago
|
||
Comment 28•2 years ago
|
||
Comment 29•2 years ago
|
||
Comment 30•2 years ago
|
||
Comment 31•2 years ago
|
||
Comment 32•2 years ago
|
||
Adding followup files that show more control
Comment 33•2 years ago
|
||
Updated•2 years ago
|
Updated•9 months ago
|
Description
•