Some PortMidiController buffer overflows and logic fixes.

* Prevent buffer overflow when SysEx messages are larger than 1024
  bytes.
* Remove logic that treats any real-time byte in a SysEx payload as a
  real-time message. PortMidi will deliver real-time messages that occur
  during a SysEx message as their own message, so I don't believe this
  logic is correct. Additionally it has a bug whereby the rest of the
  bytes following the real-time byte in the message are ignored.
* Ignore SysEx messages from controller scripts when they do not end
  with an EOX byte (0xF7). A SysEx message without an EOX will cause a
  segfault in PortMidi.

2 files changed, 18 insertions, 8 deletions

diff --git a/src/controllers/midi/portmidicontroller.cpp b/src/controllers/midi/portmidicontroller.cpp
index e5b7c1fde8..5a486e6c4f 100644
--- a/src/controllers/midi/portmidicontroller.cpp
+++ b/src/controllers/midi/portmidicontroller.cpp
@@ -203,14 +203,14 @@ bool PortMidiController::poll() {
             }
 
             // Collect bytes from PmMessage
-            int data = 0;
+            unsigned char data = 0;
             for (int shift = 0; shift < 32 && (data != MIDI_EOX); shift += 8) {
-                if ((data & 0xF8) == 0xF8) {
-                    // Handle real-time messages at any time
-                    receive(data, 0, 0);
-                } else {
-                    m_cReceiveMsg[m_cReceiveMsg_index++] = data =
-                        (m_midiBuffer[i].message >> shift) & 0xFF;
+                // TODO(rryan): This prevents buffer overflow if the sysex is
+                // larger than 1024 bytes. I don't want to radically change
+                // anything before the 2.0 release so this will do for now.
+                data = (m_midiBuffer[i].message >> shift) & 0xFF;
+                if (m_cReceiveMsg_index < MIXXX_SYSEX_BUFFER_LEN) {
+                    m_cReceiveMsg[m_cReceiveMsg_index++] = data;
                 }
             }
 
@@ -237,6 +237,15 @@ void PortMidiController::sendWord(unsigned int word) {
 }
 
 void PortMidiController::send(QByteArray data) {
+    // PortMidi does not receive a length argument for the buffer we provide to
+    // Pm_WriteSysEx. Instead, it scans for a MIDI_EOX byte to know when the
+    // message is over. If one is not provided, it will overflow the buffer and
+    // cause a segfault.
+    if (!data.endsWith(MIDI_EOX)) {
+        controllerDebug("SysEx message does not end with 0xF7 -- ignoring.");
+        return;
+    }
+
     if (m_pOutputStream) {
         PmError err = Pm_WriteSysEx(m_pOutputStream, 0, (unsigned char*)data.constData());
         if (err != pmNoError) {
diff --git a/src/controllers/midi/portmidicontroller.h b/src/controllers/midi/portmidicontroller.h
index 628f08e112..3e89792833 100644
--- a/src/controllers/midi/portmidicontroller.h
+++ b/src/controllers/midi/portmidicontroller.h
@@ -23,6 +23,7 @@
 // Mixxx completely stops responding to the controller if more than this number of messages queue up.
 //  Don't lower this (much.) The SCS.1d accumulated 500 messages in a single poll during stress-testing.
 #define MIXXX_PORTMIDI_BUFFER_LEN 1024 /**Number of MIDI messages to buffer*/
+#define MIXXX_SYSEX_BUFFER_LEN 1024 /**Length of SysEx buffer*/
 #define MIXXX_PORTMIDI_NO_DEVICE_STRING "None" /**String to display for no MIDI devices present */
 
 /** A PortMidi-based implementation of MidiController */
@@ -59,7 +60,7 @@ class PortMidiController : public MidiController {
     PmEvent m_midiBuffer[MIXXX_PORTMIDI_BUFFER_LEN];
 
     // Storage for SysEx messages
-    unsigned char m_cReceiveMsg[1024];
+    unsigned char m_cReceiveMsg[MIXXX_SYSEX_BUFFER_LEN];
     int m_cReceiveMsg_index;
     bool m_bInSysex;
 };