From 85ab34179583db06899105586e7ac2d264b437fb Mon Sep 17 00:00:00 2001
From: David Robillard <d@drobilla.net>
Date: Mon, 16 Feb 2009 00:36:11 +0000
Subject: [PATCH] Gracefully ignore illegal MIDI events.

git-svn-id: svn://localhost/ardour2/branches/3.0@4591 d708f5d6-7413-0410-9779-e7cbd77b26cf
---
 libs/evoral/evoral/MIDIEvent.hpp |  2 +-
 libs/evoral/evoral/midi_util.h   | 24 ++++++++++++++++--------
 libs/evoral/src/SMF.cpp          | 10 +++++++++-
 libs/evoral/src/Sequence.cpp     |  5 +++++
 4 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/libs/evoral/evoral/MIDIEvent.hpp b/libs/evoral/evoral/MIDIEvent.hpp
index c9541632d1..12c57a96a0 100644
--- a/libs/evoral/evoral/MIDIEvent.hpp
+++ b/libs/evoral/evoral/MIDIEvent.hpp
@@ -80,7 +80,7 @@ struct MIDIEvent : public Event<Time> {
 	inline void     set_pgm_number(uint8_t number){ this->_buf[1] = number; }
 	inline uint8_t  aftertouch()            const { return (this->_buf[1]); }
 	inline uint8_t  channel_pressure()      const { return (this->_buf[1]); }
-	inline bool     is_channel_event()      const { return (0x80 <= type()) && (type() <= 0xE0);	}
+	inline bool     is_channel_event()      const { return (0x80 <= type()) && (type() <= 0xE0); }
 	inline bool     is_smf_meta_event()     const { return this->_buf[0] == 0xFF; }
 	inline bool     is_sysex()              const { return this->_buf[0] == 0xF0
 	                                                    || this->_buf[0] == 0xF7; }
diff --git a/libs/evoral/evoral/midi_util.h b/libs/evoral/evoral/midi_util.h
index e290a4141d..8caf3fddc4 100644
--- a/libs/evoral/evoral/midi_util.h
+++ b/libs/evoral/evoral/midi_util.h
@@ -70,20 +70,22 @@ midi_event_size(uint8_t status)
 	return -1;
 }
 
-/** Return the size of the given event including the status byte
- * (which must be the first byte in \a buffer),
- * or -1 if unknown (eg sysex)
+/** Return the size of the given event including the status byte,
+ * or -1 if event is illegal.
  */
 static inline int
-midi_event_size(uint8_t* buffer)
+midi_event_size(const uint8_t* buffer)
 {
 	uint8_t status = buffer[0];
 	
-	// if we have a channel event
+	// Mask off channel if applicable
 	if (status >= 0x80 && status < 0xF0) {
-		status &= 0xF0; // mask off the channel
+		status &= 0xF0;
 	}
 	
+	// FIXME: This is not correct, read the size and verify
+	// A sysex can contain the byte MIDI_CMD_COMMON_SYSEX_END, so this
+	// is likely to result in corrupt buffers and catastrophic failure
 	if (status == MIDI_CMD_COMMON_SYSEX) {
 		int end;
 		for (end = 1; buffer[end] != MIDI_CMD_COMMON_SYSEX_END; end++) {}
@@ -94,14 +96,20 @@ midi_event_size(uint8_t* buffer)
 	}
 }
 
-/** Return true iff the given buffer is a valid MIDI event */
+/** Return true iff the given buffer is a valid MIDI event.
+ * \a len must be exactly correct for the contents of \a buffer
+ */
 static inline bool
-midi_event_is_valid(uint8_t* buffer, size_t len)
+midi_event_is_valid(const uint8_t* buffer, size_t len)
 {
 	uint8_t status = buffer[0];
 	if (status < 0x80) {
 		return false;
 	}
+	const int size = midi_event_size(buffer);
+	if (size < 0 || (size_t)size != len) {
+		return false;
+	}
 	return true;
 }
 
diff --git a/libs/evoral/src/SMF.cpp b/libs/evoral/src/SMF.cpp
index 5e17dcf6ce..17c94924f8 100644
--- a/libs/evoral/src/SMF.cpp
+++ b/libs/evoral/src/SMF.cpp
@@ -21,9 +21,10 @@
 #include <cassert>
 #include <iostream>
 #include <stdint.h>
+#include "libsmf/smf.h"
 #include "evoral/Event.hpp"
 #include "evoral/SMF.hpp"
-#include "libsmf/smf.h"
+#include "evoral/midi_util.h"
 
 using namespace std;
 
@@ -201,6 +202,8 @@ SMF::read_event(uint32_t* delta_t, uint32_t* size, uint8_t** buf) const
     	memcpy(*buf, event->midi_buffer, size_t(event_size));
     	*size = event_size;
 	
+		assert(midi_event_is_valid(*buf, *size));
+
 		/*printf("SMF::read_event:\n");
 		for (size_t i = 0; i < *size; ++i) {
 			printf("%X ", (*buf)[i]);
@@ -224,6 +227,11 @@ SMF::append_event_delta(uint32_t delta_t, uint32_t size, const uint8_t* buf)
 		printf("%X ", buf[i]);
 	} printf("\n");*/
 
+	if (!midi_event_is_valid(buf, size)) {
+		cerr << "WARNING: Ignoring illegal MIDI event" << endl;
+		return;
+	}
+
 	smf_event_t* event;
 
 	event = smf_event_new_from_pointer(buf, size);
diff --git a/libs/evoral/src/Sequence.cpp b/libs/evoral/src/Sequence.cpp
index e834182637..ee5fabccb8 100644
--- a/libs/evoral/src/Sequence.cpp
+++ b/libs/evoral/src/Sequence.cpp
@@ -580,6 +580,11 @@ Sequence<Time>::append(const Event<Time>& event)
 	assert(_notes.empty() || ev.time() >= _notes.back()->time());
 	assert(_writing);
 
+	if (!midi_event_is_valid(ev.buffer(), ev.size())) {
+		cerr << "WARNING: Ignoring illegal MIDI event" << endl;
+		return;
+	}
+
 	if (ev.is_note_on()) {
 		append_note_on_unlocked(ev.channel(), ev.time(), ev.note(), ev.velocity());
 	} else if (ev.is_note_off()) {