SMF: various backports from libsmf PR7

see also https://github.com/stump/libsmf/pull/7

* Fix validity checks of escaped data
* Handle non-EOT-terminated tracks.
* Fix buffer overflow on tempo change event
* Fix memory leaks in case loading fails
* Fix a logic errors in extract_escaped_event()
* Fix the assertion problem `is_sysex_byte(status)`
* Make libsmf more tolerant to malformed MIDI files.
  (fixes import of files generated by NoteEdit)
This commit is contained in:
Robin Gareus 2020-07-16 18:00:40 +02:00
parent 23e6dd5f6b
commit 803dab7d87
No known key found for this signature in database
GPG key ID: A090BCE02CF57F04
2 changed files with 55 additions and 21 deletions

View file

@ -77,7 +77,8 @@ next_chunk(smf_t *smf)
smf->next_chunk_offset += sizeof(struct chunk_header_struct) + GUINT32_FROM_BE(chunk->length); smf->next_chunk_offset += sizeof(struct chunk_header_struct) + GUINT32_FROM_BE(chunk->length);
if (smf->next_chunk_offset > smf->file_buffer_length) { if (smf->next_chunk_offset > smf->file_buffer_length) {
g_warning("SMF error: malformed chunk; truncated file?"); g_warning("SMF warning: malformed chunk; truncated file?");
smf->next_chunk_offset = smf->file_buffer_length;
} }
return (chunk); return (chunk);
@ -211,6 +212,8 @@ smf_extract_vlq(const unsigned char *buf, const size_t buffer_length, uint32_t *
const unsigned char *c = buf; const unsigned char *c = buf;
int i = 0; int i = 0;
assert(buffer_length > 0);
for (;; ++i) { for (;; ++i) {
if (c >= buf + buffer_length) { if (c >= buf + buffer_length) {
g_warning("End of buffer in extract_vlq()."); g_warning("End of buffer in extract_vlq().");
@ -285,7 +288,7 @@ expected_sysex_length(const unsigned char status, const unsigned char *second_by
#ifndef NDEBUG #ifndef NDEBUG
(void) status; (void) status;
#else #else
assert(status == 0xF0); assert(status == 0xF0 || status == 0xF7);
#endif #endif
if (buffer_length < 3) { if (buffer_length < 3) {
@ -293,7 +296,9 @@ expected_sysex_length(const unsigned char status, const unsigned char *second_by
return (-1); return (-1);
} }
smf_extract_vlq(second_byte, buffer_length, &sysex_length, &len); if (smf_extract_vlq(second_byte, buffer_length, &sysex_length, &len)) {
return (-1);
}
if (consumed_bytes != NULL) if (consumed_bytes != NULL)
*consumed_bytes = len; *consumed_bytes = len;
@ -464,7 +469,7 @@ extract_escaped_event(const unsigned char *buf, const size_t buffer_length, smf_
message_length = expected_escaped_length(status, c, buffer_length - 1, &vlq_length); message_length = expected_escaped_length(status, c, buffer_length - 1, &vlq_length);
if (message_length < 0) if (message_length <= 0)
return (-3); return (-3);
c += vlq_length; c += vlq_length;
@ -483,12 +488,12 @@ extract_escaped_event(const unsigned char *buf, const size_t buffer_length, smf_
memcpy(event->midi_buffer, c, message_length); memcpy(event->midi_buffer, c, message_length);
if (smf_event_is_valid(event)) { if (!smf_event_is_valid(event)) {
g_warning("Escaped event is invalid."); g_warning("Escaped event is invalid.");
return (-1); return (-1);
} }
if (smf_event_is_system_realtime(event) || smf_event_is_system_common(event)) { if (!(smf_event_is_system_realtime(event) || smf_event_is_system_common(event))) {
g_warning("Escaped event is not System Realtime nor System Common."); g_warning("Escaped event is not System Realtime nor System Common.");
} }
@ -527,11 +532,21 @@ extract_midi_event(const unsigned char *buf, const size_t buffer_length, smf_eve
return (-1); return (-1);
} }
if (is_sysex_byte(status)) if (is_sysex_byte(status)) {
if (c == buf) {
g_critical("SMF error: running status is not applicable to System Exclusive events.");
return (-2);
}
return (extract_sysex_event(buf, buffer_length, event, len, last_status)); return (extract_sysex_event(buf, buffer_length, event, len, last_status));
}
if (is_escape_byte(status)) if (is_escape_byte(status)) {
if (c == buf) {
g_critical("SMF error: running status is not applicable to Escape events.");
return (-2);
}
return (extract_escaped_event(buf, buffer_length, event, len, last_status)); return (extract_escaped_event(buf, buffer_length, event, len, last_status));
}
/* At this point, "c" points to first byte following the status byte. */ /* At this point, "c" points to first byte following the status byte. */
message_length = expected_message_length(status, c, buffer_length - (c - buf)); message_length = expected_message_length(status, c, buffer_length - (c - buf));
@ -584,6 +599,7 @@ parse_next_event(smf_track_t *track)
assert(track->next_event_offset > 0); assert(track->next_event_offset > 0);
buffer_length = track->file_buffer_length - track->next_event_offset; buffer_length = track->file_buffer_length - track->next_event_offset;
/* if there was no meta-EOT event, buffer_length can be zero. This is /* if there was no meta-EOT event, buffer_length can be zero. This is
an error in the SMF file, but it shouldn't be treated as fatal. an error in the SMF file, but it shouldn't be treated as fatal.
*/ */
@ -805,21 +821,32 @@ smf_event_is_valid(const smf_event_t *event)
static int static int
parse_mtrk_chunk(smf_track_t *track) parse_mtrk_chunk(smf_track_t *track)
{ {
smf_t *smf = track->smf;
smf_event_t *event; smf_event_t *event;
int ret = 0;
if (parse_mtrk_header(track)) if (parse_mtrk_header(track))
return (-1); return (-1);
for (;;) { for (;;) {
if (track->next_event_offset == track->file_buffer_length) {
g_warning("SMF warning: The track did not finish with the End of Track event.");
break;
}
event = parse_next_event(track); event = parse_next_event(track);
/* Couldn't parse an event? */ /* Couldn't parse an event? */
if (event == NULL || !smf_event_is_valid(event)) { if (event == NULL) {
ret = -1; g_warning("Unable to parse MIDI event; truncating track.");
if (smf_track_add_eot_delta_pulses(track, 0) != 0) {
g_critical("smf_track_add_eot_delta_pulses failed.");
return (-2);
}
break; break;
} }
assert(smf_event_is_valid(event));
if (event_is_end_of_track(event)) { if (event_is_end_of_track(event)) {
break; break;
} }
@ -842,7 +869,7 @@ parse_mtrk_chunk(smf_track_t *track)
track->file_buffer_length = 0; track->file_buffer_length = 0;
track->next_event_offset = -1; track->next_event_offset = -1;
return (ret); return (0);
} }
/** /**
@ -904,7 +931,6 @@ smf_t *
smf_load_from_memory(void *buffer, const size_t buffer_length) smf_load_from_memory(void *buffer, const size_t buffer_length)
{ {
int i; int i;
int ret;
smf_t *smf = smf_new(); smf_t *smf = smf_new();
@ -912,26 +938,30 @@ smf_load_from_memory(void *buffer, const size_t buffer_length)
smf->file_buffer_length = buffer_length; smf->file_buffer_length = buffer_length;
smf->next_chunk_offset = 0; smf->next_chunk_offset = 0;
if (parse_mthd_chunk(smf)) if (parse_mthd_chunk(smf)) {
smf_delete(smf);
return (NULL); return (NULL);
}
for (i = 1; i <= smf->expected_number_of_tracks; i++) { for (i = 1; i <= smf->expected_number_of_tracks; i++) {
smf_track_t *track = smf_track_new(); smf_track_t *track = smf_track_new();
if (track == NULL) if (track == NULL) {
smf_delete(smf);
return (NULL); return (NULL);
}
smf_add_track(smf, track); smf_add_track(smf, track);
ret = parse_mtrk_chunk(track); /* Skip unparseable chunks. */
if (parse_mtrk_chunk(track)) {
g_warning("SMF warning: Cannot load track.");
smf_track_delete(track);
break;
}
track->file_buffer = NULL; track->file_buffer = NULL;
track->file_buffer_length = 0; track->file_buffer_length = 0;
track->next_event_offset = -1; track->next_event_offset = -1;
if (ret) {
g_warning("SMF warning: Error parsing track, continuing with data loaded so far.");
break;
}
} }
if (smf->expected_number_of_tracks != smf->number_of_tracks) { if (smf->expected_number_of_tracks != smf->number_of_tracks) {

View file

@ -133,6 +133,10 @@ maybe_add_to_tempo_map(smf_event_t *event)
/* Tempo Change? */ /* Tempo Change? */
if (event->midi_buffer[1] == 0x51) { if (event->midi_buffer[1] == 0x51) {
if (event->midi_buffer_length < 6) {
g_warning("Ignoring incomplete tempo change event.");
return;
}
int ntempo = (event->midi_buffer[3] << 16) + (event->midi_buffer[4] << 8) + event->midi_buffer[5]; int ntempo = (event->midi_buffer[3] << 16) + (event->midi_buffer[4] << 8) + event->midi_buffer[5];
if (ntempo <= 0) { if (ntempo <= 0) {
g_warning("Ignoring invalid tempo change."); g_warning("Ignoring invalid tempo change.");