PNG: heap-buffer-overflow crash [@MOZ_PNG_read_filt_row_a]

RESOLVED FIXED in Firefox 49

Status

()

defect
--
critical
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: posidron, Assigned: glennrp+bmo)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla51
x86_64
All
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox48- wontfix, firefox49+ fixed, firefox-esr4549+ fixed, firefox50+ fixed, firefox51+ fixed)

Details

(Whiteboard: [adv-main49+][adv-esr45.4+])

Attachments

(7 attachments, 22 obsolete attachments)

5.14 KB, image/png
Details
5.14 KB, image/png
Details
5.14 KB, image/png
Details
1.11 KB, patch
jrmuizel
: review+
abillings
: sec-approval+
Details | Diff | Splinter Review
1.32 KB, patch
jrmuizel
: review+
abillings
: sec-approval+
Details | Diff | Splinter Review
4.82 KB, patch
jrmuizel
: review+
abillings
: sec-approval+
Details | Diff | Splinter Review
1.11 KB, patch
abillings
: sec-approval+
Details | Diff | Splinter Review
Reporter

Description

3 years ago
The following testcase crashes on en-us.linux-x86_64-asan.tar.bz2 revision 0684715b032f06273416fdc554c071ee61a889db

See attachment.

Backtrace:

==5608==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x610000321503 at pc 0x7f5a6880760e bp 0x7f5a47afdaf0 sp 0x7f5a47afdae8
READ of size 16 at 0x610000321503 thread T32 (ImgDecoder #1)
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6481][GFX1]: Unknown image format 16 (t=286.938) |[6467][GFX1]: Unknown image format 16 (t=286.437) |[6468][GFX1]: Unknown image format 16 (t=286.471) |[6469][GFX1]: Unknown image format 16 (t=286.504) |[6470][GFX1]: Unknown image format 16 (t=286.537) |[6471][GFX1]: Unknown image format 16 (t=286.588) |[6472][GFX1]: Unknown image format 16 (t=286.604) |[6473][GFX1]: Unknown image format 16 (t=286.637) |[6474][GFX1]: Unknown image format 16 (t=286.671) |[6475][GFX1]: Unknown image format 16 (t=286.704) |[6476][GFX1]: Unknown image format 16 (t=286.737) |[6477][GFX1]: Unknown image format 16 (t=286.771) |[6478][GFX1]: Unknown image format 16 (t=286.804) |[6479][GFX1]: Unknown image format 16 (t=286.854) |[6480][GFX1]: Unknown image format 16 (t=286.888) [GFX1]: Unknown image format 16
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6481][GFX1]: Unknown image format 16 (t=286.938) |[6482][GFX1]: Unknown image format 16 (t=286.971) |[6468][GFX1]: Unknown image format 16 (t=286.471) |[6469][GFX1]: 
[...]
Unknown image format 16 (t=290.904) |[6610][GFX1]: Unknown image format 16 (t=290.937) |[6611][GFX1]: Unknown image format 16 (t=290.954) |[6612][GFX1]: Unknown image format 16 (t=290.988) |[6613][GFX1]: Unknown image format 16 (t=291.021) |[6614][GFX1]: Unknown image format 16 (t=291.037) |[6615][GFX1]: Unknown image format 16 (t=291.071) [GFX1]: Unknown image format 16
    #0 0x7f5a6880760d in MOZ_PNG_read_filt_row_a /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngrutil.c:4017:31
    #1 0x7f5a687e7b02 in MOZ_PNG_read_filt_row /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngrutil.c:4170:7
    #2 0x7f5a687e7b02 in MOZ_PNG_push_proc_row /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngpread.c:905
    #3 0x7f5a687e71f6 in MOZ_PNG_proc_IDAT_data /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngpread.c:873:13
    #4 0x7f5a687df5fe in MOZ_PNG_push_read_IDAT /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngpread.c:754:7
    #5 0x7f5a687db137 in MOZ_PNG_proc_some_data /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngpread.c:115:10
    #6 0x7f5a687db137 in MOZ_PNG_process_data /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngpread.c:46
    #7 0x7f5a622b7e69 in mozilla::image::nsPNGDecoder::ReadPNGData(char const*, unsigned long) /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/decoders/nsPNGDecoder.cpp:379:3
    #8 0x7f5a622b588f in operator() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/decoders/nsPNGDecoder.cpp:362:16
    #9 0x7f5a622b588f in Lex<(lambda at /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/decoders/nsPNGDecoder.cpp:359:21)> /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/StreamingLexer.h:382
    #10 0x7f5a622b588f in mozilla::image::nsPNGDecoder::DoDecode(mozilla::image::SourceBufferIterator&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/decoders/nsPNGDecoder.cpp:358
    #11 0x7f5a6221d4ca in mozilla::image::Decoder::Decode(mozilla::NotNull<mozilla::image::IResumable*>) /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/Decoder.cpp:155:44
    #12 0x7f5a6222f996 in mozilla::image::DecodingTask::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/IDecodingTask.cpp:88:17
    #13 0x7f5a6223d332 in mozilla::image::DecodePoolWorker::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/DecodePool.cpp:187:11
    #14 0x7f5a5fa74b86 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:1068:7
    #15 0x7f5a5faf2f6c in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290:10
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6616][GFX1]: Unknown image format 16 (t=291.104) |[6617][GFX1]: Unknown image format 16 (t=291.171) |[6603][GFX1]: Unknown image format 16 (t=290.705) |[6604][GFX1]: Unknown image format 16 (t=290.738) |[6605][GFX1]: Unknown image format 16 (t=290.771) |[6606][GFX1]: Unknown image format 16 (t=290.805) |[6607][GFX1]: Unknown image format 16 (t=290.854) |[6608][GFX1]: Unknown image format 16 (t=290.871) |[6609][GFX1]: Unknown image format 16 (t=290.904) |[6610][GFX1]: Unknown image format 16 (t=290.937) |[6611][GFX1]: Unknown image format 16 (t=290.954) |[6612][GFX1]: Unknown image format 16 (t=290.988) |[6613][GFX1]: Unknown image format 16 (t=291.021) |[6614][GFX1]: Unknown image format 16 (t=291.037) |[6615][GFX1]: Unknown image format 16 (t=291.071) [GFX1]: Unknown image format 16
    #16 0x7f5a6084090c in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/glue/MessagePump.cpp:354:20
    #17 0x7f5a607b3d28 in RunInternal /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:232:3
    #18 0x7f5a607b3d28 in RunHandler /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:225
    #19 0x7f5a607b3d28 in MessageLoop::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:205
    #20 0x7f5a5fa6fea1 in nsThread::ThreadFunc(void*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:463:5
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6616][GFX1]: Unknown image format 16 (t=291.104) |[6617][GFX1]: Unknown image format 16 (t=291.171) |[6618][GFX1]: Unknown image format 16 (t=291.204) |[6604][GFX1]: Unknown image format 16 (t=290.738) |[6605][GFX1]: Unknown image format 16 (t=290.771) |[6606][GFX1]: Unknown image format 16 (t=290.805) |[6607][GFX1]: Unknown image format 16 (t=290.854) |[6608][GFX1]: Unknown image format 16 (t=290.871) |[6609][GFX1]: Unknown image format 16 (t=290.904) |[6610][GFX1]: Unknown image format 16 (t=290.937) |[6611][GFX1]: Unknown image format 16 (t=290.954) |[6612][GFX1]: Unknown image format 16 (t=290.988) |[6613][GFX1]: Unknown image format 16 (t=291.021) |[6614][GFX1]: Unknown image format 16 (t=291.037) |[6615][GFX1]: Unknown image format 16 (t=291.071) [GFX1]: Unknown image format 16
    #21 0x7f5a76ba3378 in _pt_root /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:216:5
    #22 0x7f5a7a115181 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8181)

0x610000321503 is located 10 bytes to the right of 185-byte region [0x610000321440,0x6100003214f9)
allocated by thread T32 (ImgDecoder #1) here:
    #0 0x4b247b in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:52:3
    #1 0x7f5a687f1452 in MOZ_PNG_malloc_base /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngmem.c:95:17
    #2 0x7f5a687f1452 in MOZ_PNG_malloc /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngmem.c:179
    #3 0x7f5a687f1452 in MOZ_PNG_read_start_row /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngrutil.c:4673
    #4 0x7f5a687f0e87 in MOZ_PNG_read_update_info /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngread.c:350:10
    #5 0x7f5a622b1aa3 in mozilla::image::nsPNGDecoder::info_callback(png_struct_def*, png_info_def*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/decoders/nsPNGDecoder.cpp:648:3
    #6 0x7f5a687dd51f in MOZ_PNG_push_have_info /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngpread.c:1189:7
    #7 0x7f5a687dd51f in MOZ_PNG_push_read_chunk /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngpread.c:351
    #8 0x7f5a687db12b in MOZ_PNG_proc_some_data /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngpread.c:109:10
    #9 0x7f5a687db12b in MOZ_PNG_process_data /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngpread.c:46
    #10 0x7f5a622b7e69 in mozilla::image::nsPNGDecoder::ReadPNGData(char const*, unsigned long) /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/decoders/nsPNGDecoder.cpp:379:3
    #11 0x7f5a622b588f in operator() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/decoders/nsPNGDecoder.cpp:362:16
    #12 0x7f5a622b588f in Lex<(lambda at /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/decoders/nsPNGDecoder.cpp:359:21)> /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/StreamingLexer.h:382
    #13 0x7f5a622b588f in mozilla::image::nsPNGDecoder::DoDecode(mozilla::image::SourceBufferIterator&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/decoders/nsPNGDecoder.cpp:358
    #14 0x7f5a6221d4ca in mozilla::image::Decoder::Decode(mozilla::NotNull<mozilla::image::IResumable*>) /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/Decoder.cpp:155:44
    #15 0x7f5a6222f996 in mozilla::image::DecodingTask::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/IDecodingTask.cpp:88:17
    #16 0x7f5a6223d332 in mozilla::image::DecodePoolWorker::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/DecodePool.cpp:187:11
    #17 0x7f5a5fa74b86 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:1068:7
    #18 0x7f5a5faf2f6c in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290:10
    #19 0x7f5a6084090c in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/glue/MessagePump.cpp:354:20
    #20 0x7f5a607b3d28 in RunInternal /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:232:3
    #21 0x7f5a607b3d28 in RunHandler /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:225
    #22 0x7f5a607b3d28 in MessageLoop::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:205
    #23 0x7f5a5fa6fea1 in nsThread::ThreadFunc(void*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:463:5
    #24 0x7f5a76ba3378 in _pt_root /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:216:5
    #25 0x7f5a7a115181 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8181)

Thread T32 (ImgDecoder #1) created by T0 here:
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6616][GFX1]: Unknown image format 16 (t=291.104) |[6617][GFX1]: Unknown image format 16 (t=291.171) |[6618][GFX1]: Unknown image format 16 (t=291.204) |[6619][GFX1]: Unknown image format 16 (t=291.255) |[6605][GFX1]: Unknown image format 16 (t=290.771) |[6606][GFX1]: Unknown image format 16 (t=290.805) |[6607][GFX1]: Unknown image format 16 (t=290.854) |[6608][GFX1]: Unknown image format 16 (t=290.871) |[6609][GFX1]: Unknown image format 16 (t=290.904) |[6610][GFX1]: Unknown image format 16 (t=290.937) |[6611][GFX1]: Unknown image format 16 (t=290.954) |[6612][GFX1]: Unknown image format 16 (t=290.988) |[6613][GFX1]: Unknown image format 16 (t=291.021) |[6614][GFX1]: Unknown image format 16 (t=291.037) |[6615][GFX1]: Unknown image format 16 (t=291.071) [GFX1]: Unknown image format 16
    #0 0x49a839 in __interceptor_pthread_create /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:238:3
    #1 0x7f5a76b9ff3f in _PR_CreateThread /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:457:14
    #2 0x7f5a76b9fb4a in PR_CreateThread /builds/slave/m-in-l64-asan-0000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:548:12
    #3 0x7f5a5fa7162b in nsThread::Init() /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:634:8
    #4 0x7f5a5fa78d4f in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThreadManager.cpp:253:17
    #5 0x7f5a5faf1f5e in NS_NewThread(nsIThread**, nsIRunnable*, unsigned int) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:64:5
    #6 0x7f5a6221ad98 in mozilla::image::DecodePool::DecodePool() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/DecodePool.cpp:263:19
    #7 0x7f5a6221a310 in Singleton /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/DecodePool.cpp:220:22
    #8 0x7f5a6221a310 in mozilla::image::DecodePool::Initialize() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/DecodePool.cpp:212
    #9 0x7f5a62273ce5 in mozilla::image::EnsureModuleInitialized() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/build/nsImageModule.cpp:103:3
    #10 0x7f5a621d5b01 in imgLoader::CreateImageLoader() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/imgLoader.cpp:1147:3
    #11 0x7f5a621d5dd8 in imgLoader::NormalLoader() /builds/slave/m-in-l64-asan-0000000000000000/build/src/image/imgLoader.cpp:1159:21
    #12 0x7f5a6236dc76 in GetImgLoaderForDocument /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/base/nsContentUtils.cpp:3136:22
    #13 0x7f5a6236dc76 in nsContentUtils::LoadImage(nsIURI*, nsINode*, nsIDocument*, nsIPrincipal*, nsIURI*, mozilla::net::ReferrerPolicy, imgINotificationObserver*, int, nsAString_internal const&, imgRequestProxy**, unsigned int) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/base/nsContentUtils.cpp:3188
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6616][GFX1]: Unknown image format 16 (t=291.104) |[6617][GFX1]: Unknown image format 16 (t=291.171) |[6618][GFX1]: Unknown image format 16 (t=291.204) |[6619][GFX1]: Unknown image format 16 (t=291.255) |[6620][GFX1]: Unknown image format 16 (t=291.288) |[6606][GFX1]: Unknown image format 16 (t=290.805) |[6607][GFX1]: Unknown image format 16 (t=290.854) |[6608][GFX1]: Unknown image format 16 (t=290.871) |[6609][GFX1]: Unknown image format 16 (t=290.904) |[6610][GFX1]: Unknown image format 16 (t=290.937) |[6611][GFX1]: Unknown image format 16 (t=290.954) |[6612][GFX1]: Unknown image format 16 (t=290.988) |[6613][GFX1]: Unknown image format 16 (t=291.021) |[6614][GFX1]: Unknown image format 16 (t=291.037) |[6615][GFX1]: Unknown image format 16 (t=291.071) [GFX1]: Unknown image format 16
    #14 0x7f5a665abe33 in mozilla::css::ImageLoader::LoadImage(nsIURI*, nsIPrincipal*, nsIURI*, mozilla::css::ImageValue*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/style/ImageLoader.cpp:262:17
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6616][GFX1]: Unknown image format 16 (t=291.104) |[6617][GFX1]: Unknown image format 16 (t=291.171) |[6618][GFX1]: Unknown image format 16 (t=291.204) |[6619][GFX1]: Unknown image format 16 (t=291.255) |[6620][GFX1]: Unknown image format 16 (t=291.288) |[6621][GFX1]: Unknown image format 16 (t=291.338) |[6607][GFX1]: Unknown image format 16 (t=290.854) |[6608][GFX1]: Unknown image format 16 (t=290.871) |[6609][GFX1]: Unknown image format 16 (t=290.904) |[6610][GFX1]: Unknown image format 16 (t=290.937) |[6611][GFX1]: Unknown image format 16 (t=290.954) |[6612][GFX1]: Unknown image format 16 (t=290.988) |[6613][GFX1]: Unknown image format 16 (t=291.021) |[6614][GFX1]: Unknown image format 16 (t=291.037) |[6615][GFX1]: Unknown image format 16 (t=291.071) [GFX1]: Unknown image format 16
    #15 0x7f5a66669696 in mozilla::css::ImageValue::ImageValue(nsIURI*, nsStringBuffer*, nsIURI*, nsIPrincipal*, nsIDocument*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/style/nsCSSValue.cpp:2655:3
    #16 0x7f5a66663365 in nsCSSValue::StartImageLoad(nsIDocument*) const /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/style/nsCSSValue.cpp:746:9
    #17 0x7f5a666c1ea9 in TryToStartImageLoadOnValue(nsCSSValue const&, nsIDocument*, nsStyleContext*, nsCSSProperty, bool) /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/style/nsCSSDataBlock.cpp:83:5
    #18 0x7f5a666c1d23 in TryToStartImageLoad(nsCSSValue const&, nsIDocument*, nsStyleContext*, nsCSSProperty, bool) /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/style/nsCSSDataBlock.cpp:118:7
    #19 0x7f5a66619e0d in MapSinglePropertyInto(nsCSSProperty, nsCSSValue const*, nsCSSValue*, nsRuleData*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/style/nsCSSDataBlock.cpp:171:5
    #20 0x7f5a66618f70 in nsCSSCompressedDataBlock::MapRuleInfoInto(nsRuleData*) const /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/style/nsCSSDataBlock.cpp:318:9
    #21 0x7f5a6657cb3c in mozilla::css::Declaration::MapRuleInfoInto(nsRuleData*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/style/Declaration.cpp:98:3
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6616][GFX1]: Unknown image format 16 (t=291.104) |[6617][GFX1]: Unknown image format 16 (t=291.171) |[6618][GFX1]: Unknown image format 16 (t=291.204) |[6619][GFX1]: Unknown image format 16 (t=291.255) |[6620][GFX1]: Unknown image format 16 (t=291.288) |[6621][GFX1]: Unknown image format 16 (t=291.338) |[6622][GFX1]: Unknown image format 16 (t=291.371) |[6608][GFX1]: Unknown image format 16 (t=290.871) |[6609][GFX1]: Unknown image format 16 (t=290.904) |[6610][GFX1]: Unknown image format 16 (t=290.937) |[6611][GFX1]: Unknown image format 16 (t=290.954) |[6612][GFX1]: Unknown image format 16 (t=290.988) |[6613][GFX1]: Unknown image format 16 (t=291.021) |[6614][GFX1]: Unknown image format 16 (t=291.037) |[6615][GFX1]: Unknown image format 16 (t=291.071) [GFX1]: Unknown image format 16
    #22 0x7f5a6677817b in nsRuleNode::WalkRuleTree(nsStyleStructID, nsStyleContext*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/style/nsRuleNode.cpp:2354:7
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6616][GFX1]: Unknown image format 16 (t=291.104) |[6617][GFX1]: Unknown image format 16 (t=291.171) |[6618][GFX1]: Unknown image format 16 (t=291.204) |[6619][GFX1]: Unknown image format 16 (t=291.255) |[6620][GFX1]: Unknown image format 16 (t=291.288) |[6621][GFX1]: Unknown image format 16 (t=291.338) |[6622][GFX1]: Unknown image format 16 (t=291.371) |[6623][GFX1]: Unknown image format 16 (t=291.404) |[6609][GFX1]: Unknown image format 16 (t=290.904) |[6610][GFX1]: Unknown image format 16 (t=290.937) |[6611][GFX1]: Unknown image format 16 (t=290.954) |[6612][GFX1]: Unknown image format 16 (t=290.988) |[6613][GFX1]: Unknown image format 16 (t=291.021) |[6614][GFX1]: Unknown image format 16 (t=291.037) |[6615][GFX1]: Unknown image format 16 (t=291.071) [GFX1]: Unknown image format 16
    #23 0x7f5a669a4b41 in GetStyleBackground<true> /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/nsStyleStructList.h:81:1
    #24 0x7f5a669a4b41 in DoGetStyleBackground<true> /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/nsStyleStructList.h:81
    #25 0x7f5a669a4b41 in StyleBackground /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/nsStyleStructList.h:81
    #26 0x7f5a669a4b41 in StartBackgroundImageLoads /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/style/nsStyleContext.h:429
    #27 0x7f5a669a4b41 in nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsContainerFrame*, nsFrameItems&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/base/nsCSSFrameConstructor.cpp:6082
    #28 0x7f5a66992112 in ConstructFramesFromItemList /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/base/nsCSSFrameConstructor.cpp:10520:5
    #29 0x7f5a66992112 in nsCSSFrameConstructor::CreateAnonymousFrames(nsFrameConstructorState&, nsIContent*, nsContainerFrame*, PendingBinding*, nsFrameItems&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/base/nsCSSFrameConstructor.cpp:4150
    #30 0x7f5a6698e7f4 in nsCSSFrameConstructor::BeginBuildingScrollFrame(nsFrameConstructorState&, nsIContent*, nsStyleContext*, nsContainerFrame*, nsIAtom*, bool, nsContainerFrame*&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/base/nsCSSFrameConstructor.cpp:4564:3
    #31 0x7f5a6698b4b5 in nsCSSFrameConstructor::SetUpDocElementContainingBlock(nsIContent*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/base/nsCSSFrameConstructor.cpp:2891:25
    #32 0x7f5a669870ca in nsCSSFrameConstructor::ConstructDocElementFrame(mozilla::dom::Element*, nsILayoutHistoryState*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/base/nsCSSFrameConstructor.cpp:2411:3
    #33 0x7f5a669aaf97 in nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsIContent*, nsILayoutHistoryState*, bool) /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/base/nsCSSFrameConstructor.cpp:7653:7
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6616][GFX1]: Unknown image format 16 (t=291.104) |[6617][GFX1]: Unknown image format 16 (t=291.171) |[6618][GFX1]: Unknown image format 16 (t=291.204) |[6619][GFX1]: Unknown image format 16 (t=291.255) |[6620][GFX1]: Unknown image format 16 (t=291.288) |[6621][GFX1]: Unknown image format 16 (t=291.338) |[6622][GFX1]: Unknown image format 16 (t=291.371) |[6623][GFX1]: Unknown image format 16 (t=291.404) |[6624][GFX1]: Unknown image format 16 (t=291.421) |[6610][GFX1]: Unknown image format 16 (t=290.937) |[6611][GFX1]: Unknown image format 16 (t=290.954) |[6612][GFX1]: Unknown image format 16 (t=290.988) |[6613][GFX1]: Unknown image format 16 (t=291.021) |[6614][GFX1]: Unknown image format 16 (t=291.037) |[6615][GFX1]: Unknown image format 16 (t=291.071) [GFX1]: Unknown image format 16
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6616][GFX1]: Unknown image format 16 (t=291.104) |[6617][GFX1]: Unknown image format 16 (t=291.171) |[6618][GFX1]: Unknown image format 16 (t=291.204) |[6619][GFX1]: Unknown image format 16 (t=291.255) |[6620][GFX1]: Unknown image format 16 (t=291.288) |[6621][GFX1]: Unknown image format 16 (t=291.338) |[6622][GFX1]: Unknown image format 16 (t=291.371) |[6623][GFX1]: Unknown image format 16 (t=291.404) |[6624][GFX1]: Unknown image format 16 (t=291.421) |[6625][GFX1]: Unknown image format 16 (t=291.438) |[6611][GFX1]: Unknown image format 16 (t=290.954) |[6612][GFX1]: Unknown image format 16 (t=290.988) |[6613][GFX1]: Unknown image format 16 (t=291.021) |[6614][GFX1]: Unknown image format 16 (t=291.037) |[6615][GFX1]: Unknown image format 16 (t=291.071) [GFX1]: Unknown image format 16
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6616][GFX1]: Unknown image format 16 (t=291.104) |[6617][GFX1]: Unknown image format 16 (t=291.171) |[6618][GFX1]: Unknown image format 16 (t=291.204) |[6619][GFX1]: Unknown image format 16 (t=291.255) |[6620][GFX1]: Unknown image format 16 (t=291.288) |[6621][GFX1]: Unknown image format 16 (t=291.338) |[6622][GFX1]: Unknown image format 16 (t=291.371) |[6623][GFX1]: Unknown image format 16 (t=291.404) |[6624][GFX1]: Unknown image format 16 (t=291.421) |[6625][GFX1]: Unknown image format 16 (t=291.438) |[6626][GFX1]: Unknown image format 16 (t=291.455) |[6612][GFX1]: Unknown image format 16 (t=290.988) |[6613][GFX1]: Unknown image format 16 (t=291.021) |[6614][GFX1]: Unknown image format 16 (t=291.037) |[6615][GFX1]: Unknown image format 16 (t=291.071) [GFX1]: Unknown image format 16
    #34 0x7f5a66b489d7 in PresShell::Initialize(int, int) /builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/base/nsPresShell.cpp:1726:7
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6616][GFX1]: Unknown image format 16 (t=291.104) |[6617][GFX1]: Unknown image format 16 (t=291.171) |[6618][GFX1]: Unknown image format 16 (t=291.204) |[6619][GFX1]: Unknown image format 16 (t=291.255) |[6620][GFX1]: Unknown image format 16 (t=291.288) |[6621][GFX1]: Unknown image format 16 (t=291.338) |[6622][GFX1]: Unknown image format 16 (t=291.371) |[6623][GFX1]: Unknown image format 16 (t=291.404) |[6624][GFX1]: Unknown image format 16 (t=291.421) |[6625][GFX1]: Unknown image format 16 (t=291.438) |[6626][GFX1]: Unknown image format 16 (t=291.455) |[6627][GFX1]: Unknown image format 16 (t=291.488) |[6613][GFX1]: Unknown image format 16 (t=291.021) |[6614][GFX1]: Unknown image format 16 (t=291.037) |[6615][GFX1]: Unknown image format 16 (t=291.071) [GFX1]: Unknown image format 16
    #35 0x7f5a6268fbc1 in nsContentSink::StartLayout(bool) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/base/nsContentSink.cpp:1210:19
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6616][GFX1]: Unknown image format 16 (t=291.104) |[6617][GFX1]: Unknown image format 16 (t=291.171) |[6618][GFX1]: Unknown image format 16 (t=291.204) |[6619][GFX1]: Unknown image format 16 (t=291.255) |[6620][GFX1]: Unknown image format 16 (t=291.288) |[6621][GFX1]: Unknown image format 16 (t=291.338) |[6622][GFX1]: Unknown image format 16 (t=291.371) |[6623][GFX1]: Unknown image format 16 (t=291.404) |[6624][GFX1]: Unknown image format 16 (t=291.421) |[6625][GFX1]: Unknown image format 16 (t=291.438) |[6626][GFX1]: Unknown image format 16 (t=291.455) |[6627][GFX1]: Unknown image format 16 (t=291.488) |[6628][GFX1]: Unknown image format 16 (t=291.522) |[6614][GFX1]: Unknown image format 16 (t=291.037) |[6615][GFX1]: Unknown image format 16 (t=291.071) [GFX1]: Unknown image format 16
    #36 0x7f5a61a2a426 in nsHtml5TreeOpExecutor::StartLayout() /builds/slave/m-in-l64-asan-0000000000000000/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:612:3
    #37 0x7f5a61a36b7e in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**) /builds/slave/m-in-l64-asan-0000000000000000/build/src/parser/html/nsHtml5TreeOperation.cpp:990:7
    #38 0x7f5a61a27d77 in nsHtml5TreeOpExecutor::RunFlushLoop() /builds/slave/m-in-l64-asan-0000000000000000/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:448:21
    #39 0x7f5a61a2c80b in nsHtml5ExecutorFlusher::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/parser/html/nsHtml5StreamParser.cpp:128:9
    #40 0x7f5a5fa74b86 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/threads/nsThread.cpp:1068:7
    #41 0x7f5a5faf2f6c in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-in-l64-asan-0000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290:10
    #42 0x7f5a6083f4ef in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/glue/MessagePump.cpp:100:21
    #43 0x7f5a607b3d28 in RunInternal /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:232:3
    #44 0x7f5a607b3d28 in RunHandler /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:225
    #45 0x7f5a607b3d28 in MessageLoop::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:205
    #46 0x7f5a662060af in nsBaseAppShell::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/widget/nsBaseAppShell.cpp:156:3
    #47 0x7f5a6813ea91 in nsAppStartup::Run() /builds/slave/m-in-l64-asan-0000000000000000/build/src/toolkit/components/startup/nsAppStartup.cpp:284:19
Crash Annotation GraphicsCriticalError: |[0][GFX1]: Unknown image format 16 (t=3.91881) |[6616][GFX1]: Unknown image format 16 (t=291.104) |[6617][GFX1]: Unknown image format 16 (t=291.171) |[6618][GFX1]: Unknown image format 16 (t=291.204) |[6619][GFX1]: Unknown image format 16 (t=291.255) |[6620][GFX1]: Unknown image format 16 (t=291.288) |[6621][GFX1]: Unknown image format 16 (t=291.338) |[6622][GFX1]: Unknown image format 16 (t=291.371) |[6623][GFX1]: Unknown image format 16 (t=291.404) |[6624][GFX1]: Unknown image format 16 (t=291.421) |[6625][GFX1]: Unknown image format 16 (t=291.438) |[6626][GFX1]: Unknown image format 16 (t=291.455) |[6627][GFX1]: Unknown image format 16 (t=291.488) |[6628][GFX1]: Unknown image format 16 (t=291.522) |[6629][GFX1]: Unknown image format 16 (t=291.554) |[6615][GFX1]: Unknown image format 16 (t=291.071) [GFX1]: Unknown image format 16
    #48 0x7f5a6828bbd3 in XREMain::XRE_mainRun() /builds/slave/m-in-l64-asan-0000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4213:10
    #49 0x7f5a6828d173 in XREMain::XRE_main(int, char**, nsXREAppData const*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4332:8
    #50 0x7f5a6828e04a in XRE_main /builds/slave/m-in-l64-asan-0000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4423:16
    #51 0x4dfb47 in do_main /builds/slave/m-in-l64-asan-0000000000000000/build/src/browser/app/nsBrowserApp.cpp:251:10
    #52 0x4dfb47 in main /builds/slave/m-in-l64-asan-0000000000000000/build/src/browser/app/nsBrowserApp.cpp:387
    #53 0x7f5a7913dec4 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)

SUMMARY: AddressSanitizer: heap-buffer-overflow /builds/slave/m-in-l64-asan-0000000000000000/build/src/media/libpng/pngrutil.c:4017:31 in MOZ_PNG_read_filt_row_a
Shadow bytes around the buggy address:
  0x0c208005c250: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c208005c260: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c208005c270: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c208005c280: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c208005c290: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01
=>0x0c208005c2a0:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c208005c2b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c208005c2c0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c208005c2d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c208005c2e0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c208005c2f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
Reporter

Comment 1

3 years ago
Posted file Testcase (obsolete) —
Assignee

Comment 2

3 years ago
Nightly just shows the "image cannot be displayed..." window, and this is
in the log:

[ImgDecoder #1]: W/PNGDecoder libpng warning: Invalid color type in IHDR
[ImgDecoder #1]: E/PNGDecoder libpng error: Invalid IHDR data
[ImgDecoder #2]: W/PNGDecoder libpng warning: Invalid color type in IHDR
[ImgDecoder #2]: E/PNGDecoder libpng error: Invalid IHDR data

when I did
cp data_1_output_Output.txt fuzzed.png
and then tried to view fuzzed.png

Other applications (pngcheck, pngcrush) also report that the
color type is invalid.

Not sure why you got a crash instead of a png_error().

Recent change to nsPNGDecoder.cpp

previously:

  if (setjmp(png_jmpbuf(mPNG))) {
  
    // We exited early. If mSuccessfulEarlyFinish isn't true, then we
    // encountered an error. We might not really know what caused it, but it
    // makes more sense to blame the data.
    if (!mSuccessfulEarlyFinish && !HasError()) {
      PostDataError();
    }

    png_destroy_read_struct(&mPNG, &mInfo, nullptr);
    return;
  }

now:

// libpng uses setjmp/longjmp for error handling.
  if (setjmp(png_jmpbuf(mPNG))) {
    return Transition::TerminateFailure();
  }

The cleanup is gone; perhaps that explains the behavior
after the error in IHDR was encountered.  At least there
may be a memory leak of mPNG.
Assignee

Updated

3 years ago
Assignee: nobody → glennrp+bmo
Status: NEW → ASSIGNED
Assignee

Comment 3

3 years ago
I'm unable to reproduce this behavior, neither with my own asan build nor with one on the try-server:

http://archive.mozilla.org/pub/firefox/try-builds/ryanvm@gmail.com-18b21945f8d6303ca41be2c9c4f9bd7e53364cab/try-linux64-asan-debug/
Group: core-security → gfx-core-security
Assignee

Comment 4

3 years ago
(In reply to Glenn Randers-Pehrson from comment #2)

> The cleanup is gone; perhaps that explains the behavior
> after the error in IHDR was encountered.  At least there
> may be a memory leak of mPNG.

I've verified with some temporary logging that the mPNG
structure does get properly destroyed so there's not a leak.

There seems to be nothing more for me to do here.
Assignee: glennrp+bmo → nobody
Status: ASSIGNED → NEW
Reporter

Comment 5

3 years ago
I also saw this only one time to happen but the fact that it was a ASan issue, lead me to post it here, cause there are no false-positive there.
Assignee

Comment 6

3 years ago
Have you still got the relevant build that you mentioned: en-us.linux-x86_64-asan.tar.bz2 revision 0684715b032f06273416fdc554c071ee61a889db ?  Is it available somewhere?

Comment 7

3 years ago
testcase
Posted image test_case.png
This repro's for me on inbound ASan builds with this test case.
Attachment #8773580 - Attachment is obsolete: true
Blocks: grizzly
Flags: in-testsuite?
Assignee

Comment 8

3 years ago
For me, the new test_case.png crashes the tab under Nightly, but my own asan build correctly reports the error and does not generate an asan dump.  In this case, the error is in an fdAT chunk, which libpng handles as an "ancillary" chunk and issues a png_warning and continues on.  In the original test_case, there is also an error in the IHDR chunk (invalid colortype) which causes libpng to abort processing the image before it gets to the bad fdAT chunk.
Assignee

Comment 9

3 years ago
Landing the patch in bug #872626 will fix this.  Currently when a bad fcTL or fdAT APNG chunk is found, the decoder continues with the bad data.  The patch will cause the decoder to discontinue with the APNG, which should be safer.
Assignee: nobody → glennrp+bmo
Assignee

Updated

3 years ago
Status: NEW → ASSIGNED
OS: Mac OS X → All
Assignee

Comment 10

3 years ago
Posted patch v00-872626-crashtest (obsolete) — Splinter Review
test_case.png converted to media/libpng/crashtests/872626-1.html
Attachment #8777180 - Flags: review?(jmuizelaar)
Assignee

Comment 11

3 years ago
@ryanvm, this is the test case you requested over in bug #872626
Flags: needinfo?(ryanvm)
So we intend to land this sec-critical fix in a non-sensitive bug? Al, what's the right practice here?
Flags: needinfo?(ryanvm) → needinfo?(abillings)
AFAIK, this is not a chemspill situation, we can let it ride the train from 49
Assignee

Comment 14

3 years ago
This is the same as the v10 patch in bug #872626 with an indentation error fixed
Assignee

Comment 15

3 years ago
Renaming the crashtest
Attachment #8777180 - Attachment is obsolete: true
Attachment #8777180 - Flags: review?(jmuizelaar)
Assignee

Comment 16

3 years ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #12)
> So we intend to land this sec-critical fix in a non-sensitive bug?

I've moved the patch over here. Works for me locally but probably ought to do another "try" to be sure.
Flags: needinfo?(ryanvm)
Assignee

Updated

3 years ago
Attachment #8777283 - Flags: review?(jmuizelaar)
Assignee

Updated

3 years ago
Attachment #8777286 - Flags: review?(jmuizelaar)
Comment on attachment 8777286 [details] [diff] [review]
v00-1288588-part02-apng-bad-crc-crashtest

This is missing a corresponding crashtest.list entry.
Flags: needinfo?(ryanvm)
Attachment #8777286 - Flags: feedback-

Comment 18

3 years ago
testcase
The same file, with the same incorrect content, but with CRC fixed.

Please test this one too.
(In reply to Ryan VanderMeulen [:RyanVM] from comment #12)
> So we intend to land this sec-critical fix in a non-sensitive bug? Al,
> what's the right practice here?

You'd get sec-approval here and land through the cover bug (if it is actually a cover bug). Fill out the sec-approval? template here and get approval here to land there. This happens often enough.

If the other bug is a fix for an issue that also happens to cover this, I'd just land it. The main issue is that if we want it on branches, people will wonder why we're porting a non-security bug to branches later but that is fine.
Flags: needinfo?(abillings)
Assignee

Comment 20

3 years ago
Added test_case_good_crc to crashtests and updated crashtests.list
Attachment #8777286 - Attachment is obsolete: true
Attachment #8777286 - Flags: review?(jmuizelaar)
Assignee

Updated

3 years ago
Attachment #8777496 - Flags: review?(jmuizelaar)
Assignee

Comment 21

3 years ago
Patch for Firefox-49 (beta)
Assignee

Comment 22

3 years ago
Sadly, although the patched asan build successfully handles the "bad crc" test, the tab crashes on the "good crc" test.  Thanks Max.
Assignee

Updated

3 years ago
Attachment #8777283 - Flags: review?(jmuizelaar) → review-

Comment 23

3 years ago
I think we should look into frame_info_callback(). We read frame dimensions there, but make no attempt to validate them:

  const IntRect frameRect(png_get_next_frame_x_offset(png_ptr, decoder->mInfo),
                          png_get_next_frame_y_offset(png_ptr, decoder->mInfo),
                          png_get_next_frame_width(png_ptr, decoder->mInfo),
                          png_get_next_frame_height(png_ptr, decoder->mInfo));

All frames in test_case.png are the same rectangle: (0,0,16,16) except one frame's buggy header is set as (0,0,0,16) and obviously creating new bitmap with width=0 could cause all kinds of memory problems.
Assignee

Comment 24

3 years ago
(In reply to Max Stepin from comment #23)
> I think we should look into frame_info_callback(). We read frame dimensions
> there, but make no attempt to validate them

Yes.  The inputs should be validated in pngget.c.  The APNG specification
tells us the valid ranges.
Assignee

Comment 25

3 years ago
It looks to me as though they are supposed to be checked properly in png_ensure_fcTL_is_valid(), in pngset.c; we need to find out why that isn't happening.
Comment on attachment 8777496 [details] [diff] [review]
v01-1288588-part02-apng-bad-crc-crashtests

Review of attachment 8777496 [details] [diff] [review]:
-----------------------------------------------------------------

Do you need to include the PNG inline? It seems like we might as well have it on the side.

It's also nice to have more verbose test names that include something about what's being tested.
Attachment #8777496 - Flags: review?(jmuizelaar) → review+
Assignee

Comment 27

3 years ago
Check dimensions for 0
Assignee

Comment 28

3 years ago
Attachment #8777583 - Attachment is obsolete: true
Assignee

Comment 30

3 years ago
Please "try" parts 01,02,02,04 together.
Flags: needinfo?(ryanvm)
Assignee

Comment 31

3 years ago
Note that builds using the system libpng+apng_patch will still be vulnerable, unless we also put a check in nsPNGDecoder.cpp after the calls to png_get_next_frame_width|height().
Assignee

Comment 33

3 years ago
Attachment #8777283 - Attachment is obsolete: true
Attachment #8777497 - Attachment is obsolete: true
Assignee

Comment 35

3 years ago
(In reply to comment #31)
> Note that builds using the system libpng+apng_patch will still be
> vulnerable, unless we also put a check in nsPNGDecoder.cpp after the calls
> to png_get_next_frame_width|height().

The patch set now takes care of this.
Assignee

Comment 36

3 years ago
Rebased after bug #1240665 landed.
Attachment #8777627 - Attachment is obsolete: true
See Also: → 872626

Comment 37

3 years ago
testcase
Posted image zero_width_frame.png
(In reply to Jeff Muizelaar [:jrmuizel] from comment #26)

> It's also nice to have more verbose test names that include something about
> what's being tested.

Here's more minimal test (only width=0 at one frame, nothing else, good crc) with a verbose name.

It should probably go into image/test/crashtests directory.
Assignee

Comment 38

3 years ago
I think we only need to fix the width==0 or height==0 issue here.  The change to CRC handling can be left to bug #872676; that only hides this bug.  I'm preparing a new set of patches now.
Assignee

Comment 39

3 years ago
Attachment #8777848 - Attachment is obsolete: true
Assignee

Comment 40

3 years ago
Please include part05 in the try run
Assignee

Comment 41

3 years ago
Only check width|height
Attachment #8777628 - Attachment is obsolete: true
Assignee

Comment 42

3 years ago
Only check for width|height == 0
Attachment #8777629 - Attachment is obsolete: true
Assignee

Comment 43

3 years ago
Added Max' zero_width_frame.png to image/test/crashtests as zero_width_fcTL_chunk.png
Attachment #8777496 - Attachment is obsolete: true
Assignee

Updated

3 years ago
Attachment #8778411 - Flags: review?(jmuizelaar)
Assignee

Updated

3 years ago
Attachment #8778529 - Flags: review?(jmuizelaar)
Assignee

Updated

3 years ago
Attachment #8777593 - Flags: review?(jmuizelaar)
Assignee

Updated

3 years ago
Attachment #8777595 - Flags: review?(jmuizelaar)
Assignee

Updated

3 years ago
Attachment #8778414 - Flags: review?(jmuizelaar)
Assignee

Updated

3 years ago
Attachment #8778415 - Flags: review?(jmuizelaar)
Assignee

Updated

3 years ago
Attachment #8778416 - Flags: review?(jmuizelaar)
Assignee

Comment 44

3 years ago
Improved the error message in pngset.c
Attachment #8777593 - Attachment is obsolete: true
Attachment #8777593 - Flags: review?(jmuizelaar)
Attachment #8778579 - Flags: review?(jmuizelaar)
Assignee

Updated

3 years ago
Attachment #8777583 - Attachment is obsolete: false
Assignee

Updated

3 years ago
Attachment #8778579 - Attachment is obsolete: true
Attachment #8778579 - Flags: review?(jmuizelaar)
Assignee

Comment 45

3 years ago
Comment on attachment 8777583 [details] [diff] [review]
v00-1288588-part03-apng-bad-crc-pngset

Phooey, the v02-part03 patch was for after libpng-1.6.24 lands, so I've retracted it for now.
Attachment #8777583 - Attachment is obsolete: true
Assignee

Comment 46

3 years ago
Comment on attachment 8777593 [details] [diff] [review]
v01-1288588-part03-apng-bad-crc-pngset

Restored the v01-part03 patch for now.
Attachment #8777593 - Attachment is obsolete: false
Attachment #8777593 - Flags: review?(jmuizelaar)
Assignee

Comment 47

3 years ago
Several of the parts (03, 04, 05) will need to be rebased after bug #1291986 (libpng-1.6.24) lands, and vice versa, the patches in bug #1291986 will have to be rebased if this one lands first.  I can deal with that either way.
Assignee

Comment 48

3 years ago
Rebased after nsPNGDecoder changed
Attachment #8778411 - Attachment is obsolete: true
Attachment #8778411 - Flags: review?(jmuizelaar)
Attachment #8778660 - Flags: review?(jmuizelaar)
Attachment #8778529 - Flags: review?(jmuizelaar) → review+
Attachment #8778415 - Flags: review?(jmuizelaar) → review+
Attachment #8778660 - Flags: review?(jmuizelaar) → review+
Attachment #8778416 - Flags: review?(jmuizelaar) → review+
Attachment #8778414 - Flags: review?(jmuizelaar) → review+
Attachment #8777593 - Flags: review?(jmuizelaar) → review+
Attachment #8777595 - Flags: review?(jmuizelaar) → review+
Assignee

Comment 49

3 years ago
Comment on attachment 8777595 [details] [diff] [review]
v00-1288588-part04-apng-bad-crc-apng-patch

apng.patch was fixed by landing bug #1291986
Attachment #8777595 - Attachment is obsolete: true
Assignee

Comment 50

3 years ago
Comment on attachment 8777593 [details] [diff] [review]
v01-1288588-part03-apng-bad-crc-pngset

pngset.c was fixed by landing bug #1291986
Attachment #8777593 - Attachment is obsolete: true
Assignee

Comment 51

3 years ago
Rebase after landing bug #1291986
Assignee

Comment 52

3 years ago
Rebase after landing bug #1291986
Assignee

Comment 53

3 years ago
Rebase after landing bug #1291986
See Also: 872626
Assignee

Comment 54

3 years ago
Combined patches into one.
Attachment #8778414 - Attachment is obsolete: true
Attachment #8778529 - Attachment is obsolete: true
Attachment #8778660 - Attachment is obsolete: true
Attachment #8780129 - Attachment is obsolete: true
Attachment #8780130 - Attachment is obsolete: true
Attachment #8780131 - Attachment is obsolete: true
Attachment #8780243 - Flags: review?(jmuizelaar)
Assignee

Updated

3 years ago
Attachment #8780243 - Attachment is obsolete: true
Attachment #8780243 - Flags: review?(jmuizelaar)
Assignee

Comment 55

3 years ago
fixed a typo
Attachment #8780252 - Flags: review?(jmuizelaar)
Attachment #8780252 - Flags: review?(jmuizelaar) → review+
Assignee

Comment 57

3 years ago
Comment on attachment 8778415 [details] [diff] [review]
v02-1288588-beta-apng-bad-fcTL-pngdecoder

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

trivially

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

yes

Which older supported branches are affected by this flaw?

all

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

yes

How likely is this patch to cause regressions; how much testing does it need?

unlikely
Attachment #8778415 - Flags: sec-approval?
Assignee

Comment 58

3 years ago
Comment on attachment 8778416 [details] [diff] [review]
v01-1288588-aurora-apng-bad-fcTL-pngdecoder

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

trivially

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

yes

Which older supported branches are affected by this flaw?

all

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

yes

How likely is this patch to cause regressions; how much testing does it need?

unlikely
Attachment #8778416 - Flags: sec-approval?
Assignee

Comment 59

3 years ago
Comment on attachment 8780252 [details] [diff] [review]
v01-1288588-apng-bad-fcTL (combined)

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Which older supported branches are affected by this flaw?

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

How likely is this patch to cause regressions; how much testing does it need?
Attachment #8780252 - Flags: sec-approval?
Assignee

Updated

3 years ago
Attachment #8780252 - Flags: sec-approval?
Assignee

Comment 60

3 years ago
Comment on attachment 8780252 [details] [diff] [review]
v01-1288588-apng-bad-fcTL (combined)

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

trivially

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

yes

Which older supported branches are affected by this flaw?

all

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

yes

How likely is this patch to cause regressions; how much testing does it need?

unlikely
Attachment #8780252 - Flags: sec-approval?
I don't think we normally get sec-approval on each part of a multi-part patch. :-)
Attachment #8778415 - Flags: sec-approval? → sec-approval+
Attachment #8778416 - Flags: sec-approval? → sec-approval+
Attachment #8780252 - Flags: sec-approval? → sec-approval+
Assignee

Comment 62

3 years ago
Please checkin the "combined" patch to trunk, "aurora" to aurora, and "beta" to beta.
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/8fd07f16556c

This needs an esr45 patch too, or will the beta patch apply cleanly?
Flags: needinfo?(glennrp+bmo)
Keywords: checkin-needed
Assignee

Comment 64

3 years ago
Flags: needinfo?(glennrp+bmo)
The branch patches still need to go through the usual uplift approval process too. Can you please do the requests?
Assignee

Comment 66

3 years ago
Comment on attachment 8780801 [details] [diff] [review]
v00-1288588-esr45-apng-bad-fcTL-pngdecoder

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

trivially

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

yes

Which older supported branches are affected by this flaw?

all

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

yes

How likely is this patch to cause regressions; how much testing does it need?

very unlikely
Attachment #8780801 - Flags: sec-approval?
Assignee

Comment 67

3 years ago
Comment on attachment 8780801 [details] [diff] [review]
v00-1288588-esr45-apng-bad-fcTL-pngdecoder

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version:

trunk (51.0a1)
(also landed in the APNG patch for libpng-1.6.24 on August 4, 2016).

Risk to taking this patch (and alternatives if risky):

not risky
 
String or UUID changes made by this patch:

adds error messages "Frame width must not be 0" and "Frame height must not be 0".

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8780801 - Flags: approval-mozilla-esr45?
Assignee

Comment 68

3 years ago
Comment on attachment 8778415 [details] [diff] [review]
v02-1288588-beta-apng-bad-fcTL-pngdecoder

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: crash caused by malevolent APNG file

[Describe test coverage new/current, TreeHerder]:
TreeHerder with trunk (see comment #56).


[Risks and why]:

Little or no risk; it simply adds a test on width or height being zero and returns an error message and aborts processing the image.

[String/UUID change made/needed]:

Adds error messages "Frame width must not be 0" and "Frame height must not be 0".
Attachment #8778415 - Flags: approval-mozilla-beta?
Assignee

Comment 69

3 years ago
Comment on attachment 8778416 [details] [diff] [review]
v01-1288588-aurora-apng-bad-fcTL-pngdecoder


Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: crash caused by malevolent APNG file

[Describe test coverage new/current, TreeHerder]:
TreeHerder with trunk (see comment #56).


[Risks and why]:

Little or no risk; it simply adds a test on width or height being zero and returns an error message and aborts processing the image.

[String/UUID change made/needed]:

Adds error messages "Frame width must not be 0" and "Frame height must not be 0".
Attachment #8778416 - Flags: approval-mozilla-aurora?
Attachment #8780801 - Flags: sec-approval?
Attachment #8780801 - Flags: sec-approval+
Attachment #8780801 - Flags: approval-mozilla-esr45?
Attachment #8780801 - Flags: approval-mozilla-esr45+
Attachment #8778415 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8778416 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/8fd07f16556c
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Group: gfx-core-security → core-security-release
Reporter

Comment 73

3 years ago
May be stupid question but why are we declaring "width" and "height" as 'int' and not 'unsigned int' ?
Assignee

Comment 74

3 years ago
(In reply to Christoph Diehl [:posidron] from comment #73)
> May be stupid question but why are we declaring "width" and "height" as
> 'int' and not 'unsigned int' ?

In trunk and Aurora they are both png_uint_32 which is unsigned long
on 32-bit systems and unsigned int on 64-bit systems.  So this
seems to have been fixed already.
Assignee

Comment 75

3 years ago
There are at least two "apng" patches being distributed from SourceForge.  The one by maxst, in the "APNG" project, and which we are using in our embedded libpng, is safe.  The one by daisuken, in the "APNG patch for libpng" project, does not guard against width==0.  Therefore it's important to guard against this in nsPNGDecoder.cpp when using the system library, and therefore landing bug #1295671 is urgent, although it's currently marked "minor" importance.
Assignee

Comment 76

3 years ago
Firefox 51 is still affected, until bug #1295671 lands.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [adv-main49+][adv-esr45.4+]
Assignee

Comment 77

3 years ago
Bug #1295671 has landed, fixing the regression with unpatched non-system libpng
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Bug 1295671 is fixed and verified. Mark 51 as fixed.
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.