Christoph Diehl [:posidron] Reporter
	Description • 8 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

	Glenn Randers-Pehrson Assignee
	Comment 2 • 8 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.

	Glenn Randers-Pehrson Assignee
	Comment 3 • 8 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/

	Glenn Randers-Pehrson Assignee
	Comment 4 • 8 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.

	Christoph Diehl [:posidron] Reporter
	Comment 5 • 8 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.

	Glenn Randers-Pehrson Assignee
	Comment 6 • 8 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?

	Tyson Smith [:tsmith]
	Comment 7 • 8 years ago
testcase

This repro's for me on inbound ASan builds with this test case.

	Glenn Randers-Pehrson Assignee
	Comment 8 • 8 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.

	Glenn Randers-Pehrson Assignee
	Comment 9 • 8 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.

	Glenn Randers-Pehrson Assignee
	Comment 10 • 8 years ago

test_case.png converted to media/libpng/crashtests/872626-1.html

	Glenn Randers-Pehrson Assignee
	Comment 11 • 8 years ago

@ryanvm, this is the test case you requested over in bug #872626

	Ryan VanderMeulen [:RyanVM]
	Comment 12 • 8 years ago

So we intend to land this sec-critical fix in a non-sensitive bug? Al, what's the right practice here?

	Sylvestre Ledru [:Sylvestre]
	Comment 13 • 8 years ago

AFAIK, this is not a chemspill situation, we can let it ride the train from 49

	Glenn Randers-Pehrson Assignee
	Comment 14 • 8 years ago

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

	Glenn Randers-Pehrson Assignee
	Comment 15 • 8 years ago

Renaming the crashtest

	Glenn Randers-Pehrson Assignee
	Comment 16 • 8 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.

	Ryan VanderMeulen [:RyanVM]
	Comment 17 • 8 years ago

Comment on attachment 8777286 [details] [diff] [review]
v00-1288588-part02-apng-bad-crc-crashtest

This is missing a corresponding crashtest.list entry.

	Max Stepin
	Comment 18 • 8 years ago
testcase

The same file, with the same incorrect content, but with CRC fixed.

Please test this one too.

	Al Billings [:abillings - ex-MoCo]
	Comment 19 • 8 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? 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.

	Glenn Randers-Pehrson Assignee
	Comment 20 • 8 years ago

Added test_case_good_crc to crashtests and updated crashtests.list

	Glenn Randers-Pehrson Assignee
	Comment 21 • 8 years ago

Patch for Firefox-49 (beta)

	Glenn Randers-Pehrson Assignee
	Comment 22 • 8 years ago

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

	Max Stepin
	Comment 23 • 8 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.

	Glenn Randers-Pehrson Assignee
	Comment 24 • 8 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.

	Glenn Randers-Pehrson Assignee
	Comment 25 • 8 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.

	Jeff Muizelaar [:jrmuizel]
	Comment 26 • 8 years ago

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.

	Glenn Randers-Pehrson Assignee
	Comment 27 • 8 years ago

Check dimensions for 0

	Glenn Randers-Pehrson Assignee
	Comment 30 • 8 years ago

Please "try" parts 01,02,02,04 together.

	Glenn Randers-Pehrson Assignee
	Comment 31 • 8 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().

	Glenn Randers-Pehrson Assignee
	Comment 35 • 8 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.

	Glenn Randers-Pehrson Assignee
	Comment 36 • 8 years ago

Rebased after bug #1240665 landed.

	Max Stepin
	Comment 37 • 8 years ago
testcase

(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.

	Glenn Randers-Pehrson Assignee
	Comment 38 • 8 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.

	Glenn Randers-Pehrson Assignee
	Comment 40 • 8 years ago

Please include part05 in the try run

	Glenn Randers-Pehrson Assignee
	Comment 41 • 8 years ago

Only check width|height

	Glenn Randers-Pehrson Assignee
	Comment 42 • 8 years ago

Only check for width|height == 0

	Glenn Randers-Pehrson Assignee
	Comment 43 • 8 years ago

Added Max' zero_width_frame.png to image/test/crashtests as zero_width_fcTL_chunk.png

	Glenn Randers-Pehrson Assignee
	Comment 44 • 8 years ago

Improved the error message in pngset.c

	Glenn Randers-Pehrson Assignee
	Comment 45 • 8 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.

	Glenn Randers-Pehrson Assignee
	Comment 46 • 8 years ago

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

Restored the v01-part03 patch for now.

	Glenn Randers-Pehrson Assignee
	Comment 47 • 8 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.

	Glenn Randers-Pehrson Assignee
	Comment 48 • 8 years ago

Rebased after nsPNGDecoder changed

	Glenn Randers-Pehrson Assignee
	Comment 49 • 8 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

	Glenn Randers-Pehrson Assignee
	Comment 50 • 8 years ago

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

pngset.c was fixed by landing bug #1291986

	Glenn Randers-Pehrson Assignee
	Comment 51 • 8 years ago

Rebase after landing bug #1291986

	Glenn Randers-Pehrson Assignee
	Comment 52 • 8 years ago

Rebase after landing bug #1291986

	Glenn Randers-Pehrson Assignee
	Comment 53 • 8 years ago

Rebase after landing bug #1291986

	Glenn Randers-Pehrson Assignee
	Comment 54 • 8 years ago

Combined patches into one.

	Glenn Randers-Pehrson Assignee
	Comment 55 • 8 years ago

fixed a typo

	Ryan VanderMeulen [:RyanVM]
	Comment 56 • 8 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c663ca95bb7

	Glenn Randers-Pehrson Assignee
	Comment 57 • 8 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

	Glenn Randers-Pehrson Assignee
	Comment 58 • 8 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

	Glenn Randers-Pehrson Assignee
	Comment 59 • 8 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?

	Glenn Randers-Pehrson Assignee
	Comment 60 • 8 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

	Al Billings [:abillings - ex-MoCo]
	Comment 61 • 8 years ago

I don't think we normally get sec-approval on each part of a multi-part patch. :-)

	Glenn Randers-Pehrson Assignee
	Comment 62 • 8 years ago

Please checkin the "combined" patch to trunk, "aurora" to aurora, and "beta" to beta.

	Ryan VanderMeulen [:RyanVM]
	Comment 63 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/8fd07f16556c

This needs an esr45 patch too, or will the beta patch apply cleanly?

	Ryan VanderMeulen [:RyanVM]
	Comment 65 • 8 years ago

The branch patches still need to go through the usual uplift approval process too. Can you please do the requests?

	Glenn Randers-Pehrson Assignee
	Comment 66 • 8 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

	Glenn Randers-Pehrson Assignee
	Comment 67 • 8 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.

	Glenn Randers-Pehrson Assignee
	Comment 68 • 8 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".

	Glenn Randers-Pehrson Assignee
	Comment 69 • 8 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".

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 70 • 8 years ago

https://hg.mozilla.org/mozilla-central/rev/8fd07f16556c

	Ryan VanderMeulen [:RyanVM]
	Comment 71 • 8 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/8d65d9b7e5d2
https://hg.mozilla.org/releases/mozilla-beta/rev/9fc9fb9f8ae8

	Ryan VanderMeulen [:RyanVM]
	Comment 72 • 8 years ago

https://hg.mozilla.org/releases/mozilla-esr45/rev/9707c3423a1e

	Christoph Diehl [:posidron] Reporter
	Comment 73 • 8 years ago

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

	Glenn Randers-Pehrson Assignee
	Comment 74 • 8 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.

	Glenn Randers-Pehrson Assignee
	Comment 75 • 8 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.

	Glenn Randers-Pehrson Assignee
	Comment 76 • 8 years ago

Firefox 51 is still affected, until bug #1295671 lands.

	Glenn Randers-Pehrson Assignee
	Comment 77 • 8 years ago

Bug #1295671 has landed, fixing the regression with unpatched non-system libpng

	Gerry Chang [:gchang]
	Comment 78 • 8 years ago

Bug 1295671 is fixed and verified. Mark 51 as fixed.