Skip to content

Commit ad6d7cf

Browse files
committed
Fix bug #175:
CVE-2017-11661, CVE-2017-11662, CVE-2017-11663, CVE-2017-11664: Add a new size parameter to _WM_SetupMidiEvent() so that it knows where to stop reading, and adjust its clients properly.
1 parent 84df184 commit ad6d7cf

File tree

7 files changed

+162
-40
lines changed

7 files changed

+162
-40
lines changed

Diff for: include/internal_midi.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ extern int _WM_midi_setup_divisions(struct _mdi *mdi, uint32_t divisions);
191191

192192
extern struct _mdi * _WM_initMDI(void);
193193
extern void _WM_freeMDI(struct _mdi *mdi);
194-
extern uint32_t _WM_SetupMidiEvent(struct _mdi *mdi, uint8_t * event_data, uint8_t running_event);
194+
extern uint32_t _WM_SetupMidiEvent(struct _mdi *mdi, uint8_t * event_data, uint32_t siz, uint8_t running_event);
195195
extern void _WM_ResetToStart(struct _mdi *mdi);
196196
extern void _WM_do_pan_adjust(struct _mdi *mdi, uint8_t ch);
197197
extern void _WM_do_note_off_extra(struct _note *nte);

Diff for: src/f_hmi.c

+34-6
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,10 @@ struct _mdi *
4242
_WM_ParseNewHmi(uint8_t *hmi_data, uint32_t hmi_size) {
4343
uint32_t hmi_tmp = 0;
4444
uint8_t *hmi_base = hmi_data;
45+
uint32_t data_siz;
4546
uint16_t hmi_bpm = 0;
4647
uint16_t hmi_division = 0;
4748

48-
// uint32_t hmi_duration_secs = 0;
4949
uint32_t hmi_track_cnt = 0;
5050
uint32_t *hmi_track_offset = NULL;
5151
uint32_t i = 0;
@@ -74,8 +74,6 @@ _WM_ParseNewHmi(uint8_t *hmi_data, uint32_t hmi_size) {
7474
uint8_t channel;
7575
} *note;
7676

77-
//FIXME: This needs to be used for sanity check.
78-
UNUSED(hmi_size);
7977

8078
if (memcmp(hmi_data, "HMI-MIDISONG061595", 18)) {
8179
_WM_GLOBAL_ERROR(__FUNCTION__, __LINE__, WM_ERR_NOT_HMI, NULL, 0);
@@ -216,21 +214,35 @@ _WM_ParseNewHmi(uint8_t *hmi_data, uint32_t hmi_size) {
216214
do {
217215
hmi_data = hmi_base + hmi_track_offset[i];
218216
hmi_delta[i] = 0;
217+
if (hmi_track_offset[i] >= hmi_size) {
218+
_WM_GLOBAL_ERROR(__FUNCTION__, __LINE__, WM_ERR_NOT_HMI, "file too short", 0);
219+
goto _hmi_end;
220+
}
221+
data_siz = hmi_size - hmi_track_offset[i];
219222

220223
if (hmi_data[0] == 0xfe) {
221224
// HMI only event of some sort.
222225
if (hmi_data[1] == 0x10) {
223226
hmi_tmp = (hmi_data[4] + 5);
224227
hmi_data += hmi_tmp;
225228
hmi_track_offset[i] += hmi_tmp;
229+
hmi_tmp += 4;
226230
} else if (hmi_data[1] == 0x15) {
227231
hmi_data += 4;
228232
hmi_track_offset[i] += 4;
233+
hmi_tmp = 8;
234+
} else {
235+
hmi_tmp = 4;
229236
}
230237
hmi_data += 4;
231238
hmi_track_offset[i] += 4;
239+
if (hmi_tmp > data_siz) {
240+
_WM_GLOBAL_ERROR(__FUNCTION__, __LINE__, WM_ERR_NOT_HMI, "file too short", 0);
241+
goto _hmi_end;
242+
}
243+
data_siz -= hmi_tmp;
232244
} else {
233-
if ((setup_ret = _WM_SetupMidiEvent(hmi_mdi,hmi_data,hmi_running_event[i])) == 0) {
245+
if ((setup_ret = _WM_SetupMidiEvent(hmi_mdi,hmi_data,data_siz,hmi_running_event[i])) == 0) {
234246
goto _hmi_end;
235247
}
236248
if ((hmi_data[0] == 0xff) && (hmi_data[1] == 0x2f) && (hmi_data[2] == 0x00)) {
@@ -269,17 +281,25 @@ _WM_ParseNewHmi(uint8_t *hmi_data, uint32_t hmi_size) {
269281

270282
hmi_data += setup_ret;
271283
hmi_track_offset[i] += setup_ret;
284+
data_siz -= setup_ret;
272285

273286
note[hmi_tmp].length = 0;
274-
if (*hmi_data > 0x7f) {
287+
if (data_siz && *hmi_data > 0x7f) {
275288
do {
289+
if (!data_siz) break;
276290
note[hmi_tmp].length = (note[hmi_tmp].length << 7) | (*hmi_data & 0x7F);
277291
hmi_data++;
292+
data_siz--;
278293
hmi_track_offset[i]++;
279294
} while (*hmi_data > 0x7F);
280295
}
296+
if (!data_siz) {
297+
_WM_GLOBAL_ERROR(__FUNCTION__, __LINE__, WM_ERR_NOT_HMI, "file too short", 0);
298+
goto _hmi_end;
299+
}
281300
note[hmi_tmp].length = (note[hmi_tmp].length << 7) | (*hmi_data & 0x7F);
282301
hmi_data++;
302+
data_siz--;
283303
hmi_track_offset[i]++;
284304

285305
if (note[hmi_tmp].length) {
@@ -293,20 +313,28 @@ _WM_ParseNewHmi(uint8_t *hmi_data, uint32_t hmi_size) {
293313
} else {
294314
hmi_data += setup_ret;
295315
hmi_track_offset[i] += setup_ret;
316+
data_siz -= setup_ret;
296317
}
297318
}
298319

299320
// get track delta
300321
// hmi_delta[i] = 0; // set at start of loop
301-
if (*hmi_data > 0x7f) {
322+
if (data_siz && *hmi_data > 0x7f) {
302323
do {
324+
if (!data_siz) break;
303325
hmi_delta[i] = (hmi_delta[i] << 7) | (*hmi_data & 0x7F);
304326
hmi_data++;
327+
data_siz--;
305328
hmi_track_offset[i]++;
306329
} while (*hmi_data > 0x7F);
307330
}
331+
if (!data_siz) {
332+
_WM_GLOBAL_ERROR(__FUNCTION__, __LINE__, WM_ERR_NOT_HMI, "file too short", 0);
333+
goto _hmi_end;
334+
}
308335
hmi_delta[i] = (hmi_delta[i] << 7) | (*hmi_data & 0x7F);
309336
hmi_data++;
337+
data_siz--;
310338
hmi_track_offset[i]++;
311339
} while (!hmi_delta[i]);
312340
if ((!smallest_delta) || (smallest_delta > hmi_delta[i])) {

Diff for: src/f_hmp.c

+13-2
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ _WM_ParseNewHmp(uint8_t *hmp_data, uint32_t hmp_size) {
235235

236236
// goto start of next chunk
237237
hmp_data = hmp_chunk[i] + chunk_length[i];
238+
chunk_length[i] -= chunk_ofs[i];
238239
hmp_chunk[i] += chunk_ofs[i]++;
239240
chunk_end[i] = 0;
240241
}
@@ -273,17 +274,19 @@ _WM_ParseNewHmp(uint8_t *hmp_data, uint32_t hmp_size) {
273274
// Reserved for loop markers
274275
// TODO: still deciding what to do about these
275276
hmp_chunk[i] += 3;
277+
chunk_length[i] -= 3;
276278
} else {
277279
uint32_t setup_ret = 0;
278280

279-
if ((setup_ret = _WM_SetupMidiEvent(hmp_mdi, hmp_chunk[i], 0)) == 0) {
281+
if ((setup_ret = _WM_SetupMidiEvent(hmp_mdi, hmp_chunk[i], chunk_length[i], 0)) == 0) {
280282
goto _hmp_end;
281283
}
282284

283285
if ((hmp_chunk[i][0] == 0xff) && (hmp_chunk[i][1] == 0x2f) && (hmp_chunk[i][2] == 0x00)) {
284286
/* End of Chunk */
285287
end_of_chunks++;
286288
chunk_end[i] = 1;
289+
chunk_length[i] -= 3;
287290
hmp_chunk[i] += 3;
288291
goto NEXT_CHUNK;
289292
} else if ((hmp_chunk[i][0] == 0xff) && (hmp_chunk[i][1] == 0x51) && (hmp_chunk[i][2] == 0x03)) {
@@ -296,18 +299,26 @@ _WM_ParseNewHmp(uint8_t *hmp_data, uint32_t hmp_size) {
296299
fprintf(stderr,"DEBUG: Tempo change %f\r\n", tempo_f);
297300
}
298301
hmp_chunk[i] += setup_ret;
302+
chunk_length[i] -= setup_ret;
299303
}
300304
var_len_shift = 0;
301305
chunk_delta[i] = 0;
302-
if (*hmp_chunk[i] < 0x80) {
306+
if (chunk_length[i] && *hmp_chunk[i] < 0x80) {
303307
do {
308+
if (! chunk_length[i]) break;
304309
chunk_delta[i] = chunk_delta[i] + ((*hmp_chunk[i] & 0x7F) << var_len_shift);
305310
var_len_shift += 7;
306311
hmp_chunk[i]++;
312+
chunk_length[i]--;
307313
} while (*hmp_chunk[i] < 0x80);
308314
}
315+
if (! chunk_length[i]) {
316+
_WM_GLOBAL_ERROR(__FUNCTION__, __LINE__, WM_ERR_NOT_HMP, "file too short", 0);
317+
goto _hmp_end;
318+
}
309319
chunk_delta[i] = chunk_delta[i] + ((*hmp_chunk[i] & 0x7F) << var_len_shift);
310320
hmp_chunk[i]++;
321+
chunk_length[i]--;
311322
} while (!chunk_delta[i]);
312323

313324
if ((!smallest_delta) || (smallest_delta > chunk_delta[i])) {

Diff for: src/f_midi.c

+37-18
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ _WM_ParseNewMidi(uint8_t *midi_data, uint32_t midi_size) {
4343

4444
uint32_t tmp_val;
4545
uint32_t midi_type;
46-
uint32_t track_size;
4746
uint8_t **tracks;
47+
uint32_t *track_size;
4848
uint32_t end_of_tracks = 0;
4949
uint32_t no_tracks;
5050
uint32_t i;
@@ -56,15 +56,11 @@ _WM_ParseNewMidi(uint8_t *midi_data, uint32_t midi_size) {
5656
float sample_count_f = 0.0;
5757
float sample_remainder = 0.0;
5858
uint8_t *sysex_store = NULL;
59-
// uint32_t sysex_store_len = 0;
6059

6160
uint32_t *track_delta;
6261
uint8_t *track_end;
6362
uint32_t smallest_delta = 0;
6463
uint32_t subtract_delta = 0;
65-
// uint32_t tmp_length = 0;
66-
// uint8_t current_event = 0;
67-
// uint8_t current_event_ch = 0;
6864
uint8_t *running_event;
6965
uint32_t setup_ret = 0;
7066

@@ -151,6 +147,7 @@ _WM_ParseNewMidi(uint8_t *midi_data, uint32_t midi_size) {
151147
_WM_midi_setup_divisions(mdi,divisions);
152148

153149
tracks = malloc(sizeof(uint8_t *) * no_tracks);
150+
track_size = malloc(sizeof(uint32_t) * no_tracks);
154151
track_delta = malloc(sizeof(uint32_t) * no_tracks);
155152
track_end = malloc(sizeof(uint8_t) * no_tracks);
156153
running_event = malloc(sizeof(uint8_t) * no_tracks);
@@ -168,38 +165,42 @@ _WM_ParseNewMidi(uint8_t *midi_data, uint32_t midi_size) {
168165
midi_data += 4;
169166
midi_size -= 4;
170167

171-
track_size = *midi_data++ << 24;
172-
track_size |= *midi_data++ << 16;
173-
track_size |= *midi_data++ << 8;
174-
track_size |= *midi_data++;
168+
/* track size */
169+
tmp_val = *midi_data++ << 24;
170+
tmp_val |= *midi_data++ << 16;
171+
tmp_val |= *midi_data++ << 8;
172+
tmp_val |= *midi_data++;
175173
midi_size -= 4;
176-
if (midi_size < track_size) {
174+
if (midi_size < tmp_val) {
177175
_WM_GLOBAL_ERROR(__FUNCTION__, __LINE__, WM_ERR_CORUPT, "(too short)", 0);
178176
goto _end;
179177
}
180-
if (track_size < 3) {
178+
if (tmp_val < 3) {
181179
_WM_GLOBAL_ERROR(__FUNCTION__, __LINE__, WM_ERR_CORUPT, "(bad track size)", 0);
182180
goto _end;
183181
}
184-
if ((midi_data[track_size - 3] != 0xFF)
185-
|| (midi_data[track_size - 2] != 0x2F)
186-
|| (midi_data[track_size - 1] != 0x00)) {
182+
if ((midi_data[tmp_val - 3] != 0xFF)
183+
|| (midi_data[tmp_val - 2] != 0x2F)
184+
|| (midi_data[tmp_val - 1] != 0x00)) {
187185
_WM_GLOBAL_ERROR(__FUNCTION__, __LINE__, WM_ERR_CORUPT, "(missing EOT)", 0);
188186
goto _end;
189187
}
190188
tracks[i] = midi_data;
191-
midi_data += track_size;
192-
midi_size -= track_size;
189+
track_size[i] = tmp_val;
190+
midi_data += tmp_val;
191+
midi_size -= tmp_val;
193192
track_end[i] = 0;
194193
running_event[i] = 0;
195194
track_delta[i] = 0;
196195

197196
while (*tracks[i] > 0x7F) {
198197
track_delta[i] = (track_delta[i] << 7) + (*tracks[i] & 0x7F);
199198
tracks[i]++;
199+
track_size[i]--;
200200
}
201201
track_delta[i] = (track_delta[i] << 7) + (*tracks[i] & 0x7F);
202202
tracks[i]++;
203+
track_size[i]--;
203204

204205
if (midi_type == 1 ) {
205206
if (track_delta[i] < smallest_delta) {
@@ -243,7 +244,7 @@ _WM_ParseNewMidi(uint8_t *midi_data, uint32_t midi_size) {
243244
}
244245
}
245246
do {
246-
setup_ret = _WM_SetupMidiEvent(mdi, tracks[i], running_event[i]);
247+
setup_ret = _WM_SetupMidiEvent(mdi, tracks[i], track_size[i], running_event[i]);
247248
if (setup_ret == 0) {
248249
goto _end;
249250
}
@@ -259,6 +260,7 @@ _WM_ParseNewMidi(uint8_t *midi_data, uint32_t midi_size) {
259260
end_of_tracks++;
260261
track_end[i] = 1;
261262
tracks[i] += 3;
263+
track_size[i] -= 3;
262264
goto NEXT_TRACK;
263265
} else if ((tracks[i][0] == 0xff) && (tracks[i][1] == 0x51) && (tracks[i][2] == 0x03)) {
264266
/* Tempo */
@@ -270,15 +272,23 @@ _WM_ParseNewMidi(uint8_t *midi_data, uint32_t midi_size) {
270272
}
271273
}
272274
tracks[i] += setup_ret;
275+
track_size[i] -= setup_ret;
273276

274277
if (*tracks[i] > 0x7f) {
275278
do {
279+
if (!track_size[i]) break;
276280
track_delta[i] = (track_delta[i] << 7) + (*tracks[i] & 0x7F);
277281
tracks[i]++;
282+
track_size[i]--;
278283
} while (*tracks[i] > 0x7f);
279284
}
285+
if (!track_size[i]) {
286+
_WM_GLOBAL_ERROR(__FUNCTION__, __LINE__, WM_ERR_CORUPT, "(too short)", 0);
287+
goto _end;
288+
}
280289
track_delta[i] = (track_delta[i] << 7) + (*tracks[i] & 0x7F);
281290
tracks[i]++;
291+
track_size[i]--;
282292
} while (!track_delta[i]);
283293
if ((!smallest_delta) || (smallest_delta > track_delta[i])) {
284294
smallest_delta = track_delta[i];
@@ -304,7 +314,7 @@ _WM_ParseNewMidi(uint8_t *midi_data, uint32_t midi_size) {
304314
for (i = 0; i < no_tracks; i++) {
305315
running_event[i] = 0;
306316
do {
307-
setup_ret = _WM_SetupMidiEvent(mdi, tracks[i], running_event[i]);
317+
setup_ret = _WM_SetupMidiEvent(mdi, tracks[i], track_size[i], running_event[i]);
308318
if (setup_ret == 0) {
309319
goto _end;
310320
}
@@ -329,16 +339,24 @@ _WM_ParseNewMidi(uint8_t *midi_data, uint32_t midi_size) {
329339
}
330340
}
331341
tracks[i] += setup_ret;
342+
track_size[i] -= setup_ret;
332343

333344
track_delta[i] = 0;
334345
if (*tracks[i] > 0x7f) {
335346
do {
347+
if (!track_size[i]) break;
336348
track_delta[i] = (track_delta[i] << 7) + (*tracks[i] & 0x7F);
337349
tracks[i]++;
350+
track_size[i]--;
338351
} while (*tracks[i] > 0x7f);
339352
}
353+
if (!track_size[i]) {
354+
_WM_GLOBAL_ERROR(__FUNCTION__, __LINE__, WM_ERR_CORUPT, "(too short)", 0);
355+
goto _end;
356+
}
340357
track_delta[i] = (track_delta[i] << 7) + (*tracks[i] & 0x7F);
341358
tracks[i]++;
359+
track_size[i]--;
342360

343361
sample_count_f = (((float) track_delta[i] * samples_per_delta_f)
344362
+ sample_remainder);
@@ -372,6 +390,7 @@ _end: free(sysex_store);
372390
free(track_delta);
373391
free(running_event);
374392
free(tracks);
393+
free(track_size);
375394
if (mdi->reverb) return (mdi);
376395
_WM_freeMDI(mdi);
377396
return (NULL);

Diff for: src/f_mus.c

+3-2
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ _WM_ParseNewMus(uint8_t *mus_data, uint32_t mus_size) {
6161
float tempo_f = 0.0;
6262
uint16_t mus_freq = 0;
6363
float samples_per_tick_f = 0.0;
64-
uint8_t mus_event[] = { 0, 0, 0, 0 };
64+
#define MUS_SZ 4
65+
uint8_t mus_event[MUS_SZ] = { 0, 0, 0, 0 };
6566
uint8_t mus_event_size = 0;
6667
uint8_t mus_prev_vol[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
6768
uint32_t setup_ret = 0;
@@ -314,7 +315,7 @@ _WM_ParseNewMus(uint8_t *mus_data, uint32_t mus_size) {
314315
break;
315316
}
316317

317-
setup_ret = _WM_SetupMidiEvent(mus_mdi, (uint8_t *)mus_event, 0);
318+
setup_ret = _WM_SetupMidiEvent(mus_mdi, (uint8_t *)mus_event, MUS_SZ, 0);
318319
if (setup_ret == 0) {
319320
goto _mus_end;
320321
}

Diff for: src/f_xmidi.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ struct _mdi *_WM_ParseNewXmi(uint8_t *xmi_data, uint32_t xmi_size) {
267267
setup_ret = 6;
268268
goto _XMI_Next_Event;
269269
}
270-
if ((setup_ret = _WM_SetupMidiEvent(xmi_mdi,xmi_data,0)) == 0) {
270+
if ((setup_ret = _WM_SetupMidiEvent(xmi_mdi,xmi_data, xmi_size, 0)) == 0) {
271271
goto _xmi_end;
272272
}
273273

0 commit comments

Comments
 (0)