Closed Bug 1413762 Opened 3 years ago Closed 3 years ago

UBSan: shift exponent is too large [@ mozilla::image::nsGIFDecoder2::ReadImageDataBlock]

Categories

(Core :: ImageLib, defect, P2)

58 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- fixed

People

(Reporter: tsmith, Assigned: tnikkel)

References

(Blocks 1 open bug)

Details

(Keywords: csectype-undefined, testcase, Whiteboard: [gfx-noted])

Attachments

(2 files)

Attached image test_case.gif
This was found with a Firefox build built with "-fsanitize=shift"

/mozilla-central/image/decoders/nsGIFDecoder2.h:74:36: runtime error: shift exponent 72 is too large for 32-bit type 'int'
    #0 mozilla::image::nsGIFDecoder2::ClearCode() const /mozilla-central/image/decoders/nsGIFDecoder2.h:74:36
    #1 mozilla::image::nsGIFDecoder2::ReadImageDataBlock(char const*) /mozilla-central/image/decoders/nsGIFDecoder2.cpp:963:25
    #2 mozilla::image::nsGIFDecoder2::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*)::$_1::operator()(mozilla::image::nsGIFDecoder2::State, char const*, unsigned long) const /mozilla-central/image/decoders/nsGIFDecoder2.cpp:497:16
    #3 BufferedRead<(lambda at /mozilla-central/image/decoders/nsGIFDecoder2.cpp:466:21)> /mozilla-central/image/StreamingLexer.h:648:28
    #4 Lex<(lambda at /mozilla-central/image/decoders/nsGIFDecoder2.cpp:466:21)> /mozilla-central/image/StreamingLexer.h:511
    #5 mozilla::image::nsGIFDecoder2::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*) /mozilla-central/image/decoders/nsGIFDecoder2.cpp:465
    #6 mozilla::image::Decoder::Decode(mozilla::image::IResumable*) /mozilla-central/image/Decoder.cpp:130:20
    #7 mozilla::image::AnimationSurfaceProvider::Run() /mozilla-central/image/AnimationSurfaceProvider.cpp:142:36
    #8 mozilla::image::DecodePoolWorker::Run() /mozilla-central/image/DecodePool.cpp:178:23
    #9 nsThread::ProcessNextEvent(bool, bool*) /mozilla-central/xpcom/threads/nsThread.cpp:1037:14
    #10 NS_ProcessNextEvent(nsIThread*, bool) /mozilla-central/xpcom/threads/nsThreadUtils.cpp:513:10
    #11 mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /mozilla-central/ipc/glue/MessagePump.cpp:334:20
    #12 RunHandler /mozilla-central/ipc/chromium/src/base/message_loop.cc:319:3
    #13 MessageLoop::Run() /mozilla-central/ipc/chromium/src/base/message_loop.cc:299
    #14 nsThread::ThreadFunc(void*) /mozilla-central/xpcom/threads/nsThread.cpp:425:11
    #15 _pt_root /mozilla-central/nsprpub/pr/src/pthreads/ptthread.c:216:5
    #16 start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x77fb)
    #17 clone /build/glibc-CxtIbX/glibc-2.26/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Has Regression Range: --- → irrelevant
Attached patch shiftgifSplinter Review
This shouldn't cause use any problems. We check the shift value before we actually use it. We just call ClearCode before we check it and go into an error state if its not in bounds.
Assignee: nobody → tnikkel
Attachment #8924415 - Flags: review?(aosmond)
Attachment #8924415 - Flags: review?(aosmond) → review+
Priority: -- → P2
Whiteboard: [gfx-noted]
Duplicate of this bug: 1414898
Pushed by tnikkel@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fedc2d408840
Check integer shift value is reasonable before using it in gif decoder. r=aosmond
https://hg.mozilla.org/mozilla-central/rev/fedc2d408840
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.