Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix off-by-one/heap-buffer-overflow as reported by ASAN. #3948

Closed
wants to merge 1 commit into from
Closed

Fix off-by-one/heap-buffer-overflow as reported by ASAN. #3948

wants to merge 1 commit into from

Conversation

follower
Copy link
Contributor

@follower follower commented Nov 7, 2017 •
edited
Loading

Previous code didn't account for the : character when allocating memory.

It's probably not the best "solution" but it should hopefully be one less category of crash-on-close on Mac (a la #3371 & #2633).

ASAN a.k.a Clang "AddressSanitizer".

Majority of original Address Sanitizer output:

device name='Akai MPK49' endpoint name=''
=================================================================
==33025==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200005969b at pc 0x000102a50d9f bp 0x7fff5dc767d0 sp 0x7fff5dc75f58
WRITE of size 12 at 0x60200005969b thread T0
    #0 0x102a50d9e in wrap_vsprintf (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x25d9e)
    #1 0x102a516b4 in wrap_sprintf (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x266b4)
    #2 0x1021be425 in MidiApple::getFullName(unsigned int&) MidiApple.cpp:619
    #3 0x1021bdb46 in MidiApple::midiInClose(unsigned int) MidiApple.cpp:394
    #4 0x1021b7695 in MidiApple::closeDevices() MidiApple.cpp:377
    #5 0x1021b73d1 in MidiApple::~MidiApple() MidiApple.cpp:55
    #6 0x1021b78f4 in MidiApple::~MidiApple() MidiApple.cpp:54
    #7 0x1021b7938 in MidiApple::~MidiApple() MidiApple.cpp:54
    #8 0x1020a622d in Mixer::~Mixer() Mixer.cpp:189
    #9 0x1020a6a44 in Mixer::~Mixer() Mixer.cpp:167
    #10 0x1020a6a68 in Mixer::~Mixer() Mixer.cpp:167
    #11 0x1020312dc in void LmmsCore::deleteHelper<Mixer>(Mixer**) Engine.h:126
    #12 0x102030e60 in LmmsCore::destroy() Engine.cpp:94
    #13 0x102245be7 in MainWindow::~MainWindow() MainWindow.cpp:232
    #14 0x102245ef4 in MainWindow::~MainWindow() MainWindow.cpp:219
    #15 0x102245f38 in MainWindow::~MainWindow() MainWindow.cpp:219
    #16 0x104af4857 in QObject::event(QEvent*) (QtCore:x86_64+0x1fb857)
    #17 0x103849123 in QWidget::event(QEvent*) (QtWidgets:x86_64+0x70123)
    #18 0x10394be90 in QMainWindow::event(QEvent*) (QtWidgets:x86_64+0x172e90)
    #19 0x10380a4bb in QApplicationPrivate::notify_helper(QObject*, QEvent*) (QtWidgets:x86_64+0x314bb)
    #20 0x10380d744 in QApplication::notify(QObject*, QEvent*) (QtWidgets:x86_64+0x34744)
    #21 0x104accc23 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (QtCore:x86_64+0x1d3c23)
    #22 0x10ce2ca9d in QCocoaEventDispatcherPrivate::processPostedEvents() (libqcocoa.dylib:x86_64+0x21a9d)
    #23 0x10ce2d310 in QCocoaEventDispatcherPrivate::postedEventsSourceCallback(void*) (libqcocoa.dylib:x86_64+0x22310)
    #24 0x7fff91c1eb30 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ (CoreFoundation:x86_64+0x12b30)
    #25 0x7fff91c1e454 in __CFRunLoopDoSources0 (CoreFoundation:x86_64+0x12454)
    #26 0x7fff91c417f4 in __CFRunLoopRun (CoreFoundation:x86_64+0x357f4)
    #27 0x7fff91c410e1 in CFRunLoopRunSpecific (CoreFoundation:x86_64+0x350e1)
    #28 0x7fff8879feb3 in RunCurrentEventLoopInMode (HIToolbox:x86_64+0x5feb3)
    #29 0x7fff8879fb93 in ReceiveNextEventCommon (HIToolbox:x86_64+0x5fb93)
    #30 0x7fff8879fae2 in BlockUntilNextEventMatchingListInMode (HIToolbox:x86_64+0x5fae2)
    #31 0x7fff8b2d3532 in _DPSNextEvent (AppKit:x86_64+0x155532)
    #32 0x7fff8b2d2df1 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (AppKit:x86_64+0x154df1)
    #33 0x7fff8b2ca1a2 in -[NSApplication run] (AppKit:x86_64+0x14c1a2)
    #34 0x10ce2c204 in QCocoaEventDispatcher::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (libqcocoa.dylib:x86_64+0x21204)
    #35 0x104ac960c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (QtCore:x86_64+0x1d060c)
    #36 0x104acc4d9 in QCoreApplication::exec() (QtCore:x86_64+0x1d34d9)
    #37 0x101f90513 in main main.cpp:968
    #38 0x7fff8663b7e0 in start (libdyld.dylib:x86_64+0x27e0)

0x60200005969b is located 0 bytes to the right of 11-byte region [0x602000059690,0x60200005969b)
allocated by thread T0 here:
    #0 0x102a80d23 in wrap_malloc (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x55d23)
    #1 0x1021be3fb in MidiApple::getFullName(unsigned int&) MidiApple.cpp:618
    #2 0x1021bdb46 in MidiApple::midiInClose(unsigned int) MidiApple.cpp:394
    #3 0x1021b7695 in MidiApple::closeDevices() MidiApple.cpp:377
    #4 0x1021b73d1 in MidiApple::~MidiApple() MidiApple.cpp:55
    #5 0x1021b78f4 in MidiApple::~MidiApple() MidiApple.cpp:54
    #6 0x1021b7938 in MidiApple::~MidiApple() MidiApple.cpp:54
    #7 0x1020a622d in Mixer::~Mixer() Mixer.cpp:189
    #8 0x1020a6a44 in Mixer::~Mixer() Mixer.cpp:167
    #9 0x1020a6a68 in Mixer::~Mixer() Mixer.cpp:167
    #10 0x1020312dc in void LmmsCore::deleteHelper<Mixer>(Mixer**) Engine.h:126
    #11 0x102030e60 in LmmsCore::destroy() Engine.cpp:94
    #12 0x102245be7 in MainWindow::~MainWindow() MainWindow.cpp:232
    #13 0x102245ef4 in MainWindow::~MainWindow() MainWindow.cpp:219
    #14 0x102245f38 in MainWindow::~MainWindow() MainWindow.cpp:219
    #15 0x104af4857 in QObject::event(QEvent*) (QtCore:x86_64+0x1fb857)
    #16 0x103849123 in QWidget::event(QEvent*) (QtWidgets:x86_64+0x70123)
    #17 0x10394be90 in QMainWindow::event(QEvent*) (QtWidgets:x86_64+0x172e90)
    #18 0x10380a4bb in QApplicationPrivate::notify_helper(QObject*, QEvent*) (QtWidgets:x86_64+0x314bb)
    #19 0x10380d744 in QApplication::notify(QObject*, QEvent*) (QtWidgets:x86_64+0x34744)
    #20 0x104accc23 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (QtCore:x86_64+0x1d3c23)
    #21 0x10ce2ca9d in QCocoaEventDispatcherPrivate::processPostedEvents() (libqcocoa.dylib:x86_64+0x21a9d)
    #22 0x10ce2d310 in QCocoaEventDispatcherPrivate::postedEventsSourceCallback(void*) (libqcocoa.dylib:x86_64+0x22310)
    #23 0x7fff91c1eb30 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ (CoreFoundation:x86_64+0x12b30)
    #24 0x7fff91c1e454 in __CFRunLoopDoSources0 (CoreFoundation:x86_64+0x12454)
    #25 0x7fff91c417f4 in __CFRunLoopRun (CoreFoundation:x86_64+0x357f4)
    #26 0x7fff91c410e1 in CFRunLoopRunSpecific (CoreFoundation:x86_64+0x350e1)
    #27 0x7fff8879feb3 in RunCurrentEventLoopInMode (HIToolbox:x86_64+0x5feb3)
    #28 0x7fff8879fb93 in ReceiveNextEventCommon (HIToolbox:x86_64+0x5fb93)
    #29 0x7fff8879fae2 in BlockUntilNextEventMatchingListInMode (HIToolbox:x86_64+0x5fae2)

SUMMARY: AddressSanitizer: heap-buffer-overflow (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x25d9e) in wrap_vsprintf

@follower
Fix off-by-one/heap-buffer-overflow as reported by ASAN.
77f7948 
It's probably not the best "solution" but it should hopefully be
one less category of crash-on-close on Mac.

ASAN a.k.a Clang "AddressSanitizer".
This was referenced Nov 7, 2017
Open
Open
@lukas-w
Copy link
Member

lukas-w commented Nov 7, 2017

Good find. This should be refactored to just use std::string or QString. MidiApple.cpp contains three malloc calls, but not a single free…

@lukas-w
Copy link
Member

lukas-w commented Nov 7, 2017 •
edited
Loading

I'll cherry-pick this into stable-1.2, then close this PR. 👍

Edit: Done via eb09ff6

@lukas-w lukas-w closed this Nov 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@follower @lukas-w @gi0e5b06