Skip to content

Commit 685e54c

Browse files
authored
Merge pull request #1730 from FluidSynth/issue-1728
Fix a heap-based use after free during synth destruction
2 parents 771a27c + ca2f65d commit 685e54c

File tree

4 files changed

+37
-38
lines changed

4 files changed

+37
-38
lines changed

src/sfloader/fluid_defsfont.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323

2424
#include "fluid_defsfont.h"
25-
#include "fluid_sfont.h"
2625
#include "fluid_sys.h"
2726
#include "fluid_synth.h"
2827
#include "fluid_samplecache.h"
@@ -468,10 +467,10 @@ int fluid_defsfont_load(fluid_defsfont_t *defsfont, const fluid_file_callbacks_t
468467
return FLUID_FAILED;
469468
}
470469

471-
defsfont->fcbs = fcbs;
470+
defsfont->fcbs = *fcbs;
472471

473472
/* The actual loading is done in the sfont and sffile files */
474-
sfdata = fluid_sffile_open(file, fcbs);
473+
sfdata = fluid_sffile_open(file, &defsfont->fcbs);
475474

476475
if(sfdata == NULL)
477476
{
@@ -2246,7 +2245,7 @@ static int load_preset_samples(fluid_defsfont_t *defsfont, fluid_preset_t *prese
22462245
* for a preset */
22472246
if(sffile == NULL)
22482247
{
2249-
sffile = fluid_sffile_open(defsfont->filename, defsfont->fcbs);
2248+
sffile = fluid_sffile_open(defsfont->filename, &defsfont->fcbs);
22502249

22512250
if(sffile == NULL)
22522251
{

src/sfloader/fluid_defsfont.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "fluid_list.h"
3131
#include "fluid_mod.h"
3232
#include "fluid_gen.h"
33+
#include "fluid_sfont.h"
3334

3435

3536

@@ -105,7 +106,7 @@ int fluid_zone_inside_range(fluid_zone_range_t *zone_range, int key, int vel);
105106
*/
106107
struct _fluid_defsfont_t
107108
{
108-
const fluid_file_callbacks_t *fcbs; /* the file callbacks used to load this Soundfont */
109+
fluid_file_callbacks_t fcbs; /* the file callbacks used to load this Soundfont */
109110
char *filename; /* the filename of this soundfont */
110111
unsigned int samplepos; /* the position in the file at which the sample data starts */
111112
unsigned int samplesize; /* the size of the sample data in bytes */

src/sfloader/fluid_dls.cpp

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ struct fluid_dls_font
374374
{
375375
fluid_synth_t *synth;
376376
fluid_sfont_t *sfont;
377-
const fluid_file_callbacks_t *fcbs;
377+
const fluid_file_callbacks_t fcbs;
378378
uint32_t output_sample_rate; // only used for cdl eval
379379
std::string filename;
380380

@@ -384,7 +384,7 @@ struct fluid_dls_font
384384
{
385385
if(file != nullptr)
386386
{
387-
fcbs->fclose(file);
387+
fcbs.fclose(file);
388388
file = nullptr;
389389
}
390390
} };
@@ -477,7 +477,7 @@ struct fluid_dls_font
477477
// utilities
478478
void fseek(fluid_long_long_t pos, int whence)
479479
{
480-
if(fcbs->fseek(file, pos, whence) != FLUID_OK)
480+
if(fcbs.fseek(file, pos, whence) != FLUID_OK)
481481
{
482482
throw std::runtime_error{ "Failed to seek" };
483483
}
@@ -589,30 +589,30 @@ static void delete_fluid_dls_font(fluid_dls_font *dlsfont) noexcept
589589
#define READCHUNK_RAW(sf, var) \
590590
do \
591591
{ \
592-
if ((sf)->fcbs->fread(var, 8, (sf)->file) == FLUID_FAILED) \
592+
if ((sf)->fcbs.fread(var, 8, (sf)->file) == FLUID_FAILED) \
593593
throw std::runtime_error{ "Failed when reading chunk" }; \
594594
((RIFFChunk *)(var))->size = FLUID_LE32TOH(((RIFFChunk *)(var))->size); \
595595
} while (0)
596596

597597
#define READID(sf, var) \
598598
do \
599599
{ \
600-
if ((sf)->fcbs->fread(var, 4, (sf)->file) == FLUID_FAILED) \
600+
if ((sf)->fcbs.fread(var, 4, (sf)->file) == FLUID_FAILED) \
601601
throw std::runtime_error{ "Failed when reading FOURCC ID" }; \
602602
} while (0)
603603

604604
#define READ8(sf, var) \
605605
do \
606606
{ \
607-
if ((sf)->fcbs->fread(&var, 1, (sf)->file) == FLUID_FAILED) \
607+
if ((sf)->fcbs.fread(&var, 1, (sf)->file) == FLUID_FAILED) \
608608
throw std::runtime_error{ "Failed when reading byte" }; \
609609
} while (0)
610610

611611
#define READ16(sf, var) \
612612
do \
613613
{ \
614614
uint16_t _temp; \
615-
if ((sf)->fcbs->fread(&_temp, 2, (sf)->file) == FLUID_FAILED) \
615+
if ((sf)->fcbs.fread(&_temp, 2, (sf)->file) == FLUID_FAILED) \
616616
throw std::runtime_error{ "Failed when reading word" }; \
617617
(var) = FLUID_LE16TOH(_temp); \
618618
} while (0)
@@ -621,7 +621,7 @@ static void delete_fluid_dls_font(fluid_dls_font *dlsfont) noexcept
621621
do \
622622
{ \
623623
uint32_t _temp; \
624-
if ((sf)->fcbs->fread(&_temp, 4, (sf)->file) == FLUID_FAILED) \
624+
if ((sf)->fcbs.fread(&_temp, 4, (sf)->file) == FLUID_FAILED) \
625625
throw std::runtime_error{ "Failed when reading dword" }; \
626626
(var) = FLUID_LE32TOH(_temp); \
627627
} while (0)
@@ -758,7 +758,7 @@ inline std::string fluid_dls_font::read_name_from_info_entries(fluid_long_long_t
758758
std::string name;
759759
name.resize(chunk.size);
760760

761-
if(fcbs->fread(name.data(), chunk.size, file) != FLUID_OK)
761+
if(fcbs.fread(name.data(), chunk.size, file) != FLUID_OK)
762762
{
763763
throw std::runtime_error{ "Failed when reading INAM chunk" };
764764
}
@@ -815,7 +815,7 @@ inline static void READGUID(fluid_dls_font *sf, DLSID &id)
815815
READ16(sf, id.Data2);
816816
READ16(sf, id.Data3);
817817

818-
if(sf->fcbs->fread(id.Data4, 8, sf->file) == FLUID_FAILED)
818+
if(sf->fcbs.fread(id.Data4, 8, sf->file) == FLUID_FAILED)
819819
{
820820
throw std::runtime_error{ "Failed when reading GUID" };
821821
}
@@ -1456,34 +1456,33 @@ void convert_dls_connectionblock_to_art(fluid_dls_articulation &art,
14561456
// Parse the DLS structure
14571457
fluid_dls_font::fluid_dls_font(fluid_synth_t *synth,
14581458
fluid_sfont_t *sfont,
1459-
const fluid_file_callbacks_t *fcbs,
1459+
const fluid_file_callbacks_t *fcbs_in,
14601460
const char *filename,
14611461
uint32_t output_sample_rate,
14621462
bool try_mlock)
1463-
: synth(synth), sfont(sfont), fcbs(fcbs), output_sample_rate(output_sample_rate), filename(filename)
1463+
: synth(synth), sfont(sfont), fcbs(*fcbs_in), output_sample_rate(output_sample_rate), filename(filename)
14641464
{
14651465
// Get basic file information
14661466

1467-
file = fcbs->fopen(filename); // NOLINT(cppcoreguidelines-prefer-member-initializer)
1468-
1467+
file = fcbs.fopen(filename); // NOLINT(cppcoreguidelines-prefer-member-initializer)
14691468
if(file == nullptr)
14701469
{
14711470
throw std::runtime_error{ string_format("Unable to open file '%s'", filename) };
14721471
}
14731472

1474-
if(fcbs->fseek(file, 0L, SEEK_END) == FLUID_FAILED)
1473+
if(fcbs.fseek(file, 0L, SEEK_END) == FLUID_FAILED)
14751474
{
14761475
throw std::runtime_error{ "Seek to end of file failed" };
14771476
}
14781477

1479-
filesize = fcbs->ftell(file);
1478+
filesize = fcbs.ftell(file);
14801479

14811480
if(filesize == FLUID_FAILED)
14821481
{
14831482
throw std::runtime_error{ "Get end of file position failed" };
14841483
}
14851484

1486-
if(fcbs->fseek(file, 0, SEEK_SET) == FLUID_FAILED)
1485+
if(fcbs.fseek(file, 0, SEEK_SET) == FLUID_FAILED)
14871486
{
14881487
throw std::runtime_error{ "Rewind to start of file failed" };
14891488
}
@@ -1947,7 +1946,7 @@ inline bool fluid_dls_font::execute_cdls(fluid_long_long_t offset, int size)
19471946
return stack[--sp];
19481947
};
19491948

1950-
if(fcbs->fseek(file, offset, SEEK_SET) != FLUID_OK)
1949+
if(fcbs.fseek(file, offset, SEEK_SET) != FLUID_OK)
19511950
{
19521951
throw std::runtime_error{ "Failed to seek to CDL operations" };
19531952
}
@@ -2247,9 +2246,9 @@ inline void fluid_dls_font::parse_wave(fluid_long_long_t offset, fluid_dls_sampl
22472246
{
22482247
uint32_t c = remaining > sizeof(buffer) ? sizeof(buffer) : remaining;
22492248

2250-
if(fcbs->fread(buffer, c, file) != FLUID_OK)
2249+
if(fcbs.fread(buffer, c, file) != FLUID_OK)
22512250
{
2252-
FLUID_LOG(FLUID_ERR, "fcbs->fread failed when reading DLS data chunk");
2251+
FLUID_LOG(FLUID_ERR, "fcbs::fread failed when reading DLS data chunk");
22532252
throw std::exception{};
22542253
}
22552254

@@ -2750,7 +2749,7 @@ inline void fluid_dls_font::parse_pgal(fluid_long_long_t offset, int size)
27502749

27512750
drum_note_aliasing.emplace();
27522751

2753-
if(fcbs->fread(drum_note_aliasing->data(), 128, file) != FLUID_OK)
2752+
if(fcbs.fread(drum_note_aliasing->data(), 128, file) != FLUID_OK)
27542753
{
27552754
throw std::runtime_error{ "Failed to read drum note aliasing table" };
27562755
}
@@ -2844,7 +2843,7 @@ static sf_count_t sfvio_seek(sf_count_t offset, int whence, void *user_data) noe
28442843

28452844
newpos = std::clamp(newpos, 0LL, data->size);
28462845

2847-
if(data->font->fcbs->fseek(data->font->file, data->offset + newpos, SEEK_SET) == FLUID_OK)
2846+
if(data->font->fcbs.fseek(data->font->file, data->offset + newpos, SEEK_SET) == FLUID_OK)
28482847
{
28492848
data->pos = newpos;
28502849
}
@@ -2872,7 +2871,7 @@ static sf_count_t sfvio_read(void *buffer, sf_count_t count, void *user_data) no
28722871
return 0;
28732872
}
28742873

2875-
if(data->font->fcbs->fread(buffer, count, data->font->file) != FLUID_OK)
2874+
if(data->font->fcbs.fread(buffer, count, data->font->file) != FLUID_OK)
28762875
{
28772876
return 0;
28782877
}

src/synth/fluid_synth.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,16 +1220,6 @@ delete_fluid_synth(fluid_synth_t *synth)
12201220

12211221
delete_fluid_list(synth->sfont);
12221222

1223-
/* delete all the SoundFont loaders */
1224-
1225-
for(list = synth->loaders; list; list = fluid_list_next(list))
1226-
{
1227-
loader = (fluid_sfloader_t *) fluid_list_get(list);
1228-
fluid_sfloader_delete(loader);
1229-
}
1230-
1231-
delete_fluid_list(synth->loaders);
1232-
12331223
/* wait for and delete all the lazy sfont unloading timers */
12341224

12351225
for(list = synth->fonts_to_be_unloaded; list; list = fluid_list_next(list))
@@ -1243,6 +1233,16 @@ delete_fluid_synth(fluid_synth_t *synth)
12431233

12441234
delete_fluid_list(synth->fonts_to_be_unloaded);
12451235

1236+
/* delete all the SoundFont loaders */
1237+
1238+
for(list = synth->loaders; list; list = fluid_list_next(list))
1239+
{
1240+
loader = (fluid_sfloader_t *) fluid_list_get(list);
1241+
fluid_sfloader_delete(loader);
1242+
}
1243+
1244+
delete_fluid_list(synth->loaders);
1245+
12461246
if(synth->channel != NULL)
12471247
{
12481248
for(i = 0; i < synth->midi_channels; i++)

0 commit comments

Comments
 (0)