Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Should not advance a completed iterator) [@ mozilla::image::SourceBufferIterator::AdvanceOrScheduleResume]

RESOLVED FIXED in Firefox 56

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: tsmith, Assigned: aosmond)

Tracking

(Blocks 1 bug, {assertion, testcase})

Trunk
mozilla56
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox54 unaffected, firefox55 unaffected, firefox56 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(2 attachments)

Posted image test_case.ico
This sounds like it could be bad, marking s-s. Please let me know if this should be opened up.

Changeset: 16ffc1d05422a81099ce8b9b59de66dde4c8b2f0
Build ID: 20170728132457

Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Should not advance a completed iterator), at src/image/SourceBuffer.cpp:57

#0 0x7f4da398aea0 in mozilla::image::SourceBufferIterator::AdvanceOrScheduleResume(unsigned long, mozilla::image::IResumable*) src/image/SourceBuffer.cpp:54:3
#1 0x7f4da39fa3c5 in mozilla::Variant<mozilla::image::TerminalState, mozilla::image::Yield> mozilla::image::StreamingLexer<mozilla::image::nsBMPDecoder::State, 16ul>::Lex<mozilla::image::nsBMPDecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*)::$_0>(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*, mozilla::image::nsBMPDecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*)::$_0) src/image/StreamingLexer.h:477:25
#2 0x7f4da39fa10a in mozilla::image::nsBMPDecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*) src/image/decoders/nsBMPDecoder.cpp:454:17
#3 0x7f4da3937bbf in mozilla::image::Decoder::Decode(mozilla::image::IResumable*) src/image/Decoder.cpp:130:20
#4 0x7f4da3a0922b in mozilla::image::nsICODecoder::FlushContainedDecoder() src/image/decoders/nsICODecoder.cpp:700:43
#5 0x7f4da3a0c0f7 in mozilla::image::nsICODecoder::ReadBIH(char const*) src/image/decoders/nsICODecoder.cpp:429:8
#6 0x7f4da3a0b8cc in mozilla::image::nsICODecoder::SniffResource(char const*) src/image/decoders/nsICODecoder.cpp:369:12
#7 0x7f4da3a4f87d in mozilla::image::nsICODecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*)::$_4::operator()(mozilla::image::ICOState, char const*, unsigned long) const src/image/decoders/nsICODecoder.cpp:670:16
#8 0x7f4da3a4f3a5 in mozilla::Maybe<mozilla::Variant<mozilla::image::TerminalState, mozilla::image::Yield> > mozilla::image::StreamingLexer<mozilla::image::ICOState, 32ul>::BufferedRead<mozilla::image::nsICODecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*)::$_4>(mozilla::image::SourceBufferIterator&, mozilla::image::nsICODecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*)::$_4) src/image/StreamingLexer.h:637:28
#9 0x7f4da3a0e258 in mozilla::Variant<mozilla::image::TerminalState, mozilla::image::Yield> mozilla::image::StreamingLexer<mozilla::image::ICOState, 32ul>::Lex<mozilla::image::nsICODecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*)::$_4>(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*, mozilla::image::nsICODecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*)::$_4) src/image/StreamingLexer.h:500:20
#10 0x7f4da3a0deba in mozilla::image::nsICODecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*) src/image/decoders/nsICODecoder.cpp:654:17
#11 0x7f4da3937bbf in mozilla::image::Decoder::Decode(mozilla::image::IResumable*) src/image/Decoder.cpp:130:20
#12 0x7f4da3942c77 in mozilla::image::DecodedSurfaceProvider::Run() src/image/DecodedSurfaceProvider.cpp:139:34
#13 0x7f4da3963ae4 in mozilla::image::DecodePoolWorker::Run() src/image/DecodePool.cpp:178:23
#14 0x7f4da1a58a90 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1446:14
#15 0x7f4da1a5e6b0 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:480:10
#16 0x7f4da25c4704 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:339:20
#17 0x7f4da2512977 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:326:10
#18 0x7f4da2512809 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299:3
#19 0x7f4da1a50bfb in nsThread::ThreadFunc(void*) src/xpcom/threads/nsThread.cpp:506:11
#20 0x7f4dbde7d5ed in _pt_root src/nsprpub/pr/src/pthreads/ptthread.c:216:5
#21 0x7f4dc14876b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
#22 0x7f4dc05103dc in clone /build/glibc-bfm8X4/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109
Flags: in-testsuite?
Flags: needinfo?(aosmond)
Whiteboard: [gfx-noted]
Assignee: nobody → aosmond
Reviewing the path it follows without asserts, it should be safe from a security standpoint. It shouldn't try to advance the iterator (which my patch fixes) but it will just fail gracefully, and fail to decode the image.
Attachment #8892117 - Flags: review?(tnikkel)
Comment on attachment 8892117 [details] [diff] [review]
Ignore ICO resource entries which contain little or no data., v1

What if mBytesInRes lies and says we have more data then we do, do we handle it properly if there are actually less than BITMAPINFOSIZE bytes available for the resource?
(In reply to Timothy Nikkel (:tnikkel) from comment #3)
> Comment on attachment 8892117 [details] [diff] [review]
> Ignore ICO resource entries which contain little or no data., v1
> 
> What if mBytesInRes lies and says we have more data then we do, do we handle
> it properly if there are actually less than BITMAPINFOSIZE bytes available
> for the resource?

It requests mBytesInRes in the READ_RESOURCE state (typically -- if it is a BMP with an AND mask, it will request half the data, and then the request the rest in READ_MASK_ROW state). If it turns out there isn't enough, it will hit the truncated state case, which triggers terminate success immediately. If there is a usable frame, it gets shown, otherwise nothing.
Attachment #8892117 - Flags: review?(tnikkel) → review+
https://hg.mozilla.org/mozilla-central/rev/7d01cd53da02
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Group: gfx-core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.