Skip to content

Conversation

@derselbst
Copy link
Member

@derselbst derselbst commented Dec 21, 2025

Previously, the sfont load was owning the fluid_file_callbacks_t, which however might be destroyed before all sfonts using it have been destroyed.

Also, if previously a new fluid_file_callbacks_t was passed to a sfont_loader, it would have affected all previously loaded sfonts of that loader. Now, every sfont carries it's own copy of fluid_file_callbacks_t.

Resolves #1728

Repro:

  1. fluidsynth some.dls
  2. noteon 0 60 127
  3. unload 1
  4. quit

ASAN report:

==398==ERROR: AddressSanitizer: heap-use-after-free on address 0x7b658f5ebda0 at pc 0x0000004ecce5 bp 0x7b05896c3a60 sp 0x7b05896c3a58
READ of size 8 at 0x7b658f5ebda0 thread T5
    #0 0x0000004ecce4 in fluid_dls_font::on_file_exit::{lambda()#1}::operator()() const /fluidsynth/src/sfloader/fluid_dls.cpp:387
    #1 0x0000004ecce4 in void std::__invoke_impl<void, fluid_dls_font::on_file_exit::{lambda()#1}&>(std::__invoke_other, fluid_dls_font::on_file_exit::{lambda()#1}&) /usr/include/c++/15/bits/invoke.h:63
    #2 0x0000004ecce4 in std::enable_if<is_invocable_r_v<void, fluid_dls_font::on_file_exit::{lambda()#1}&>, void>::type std::__invoke_r<void, fluid_dls_font::on_file_exit::{lambda()#1}&>(fluid_dls_font::on_file_exit::{lambda()#1}&) /usr/include/c++/15/bits/invoke.h:113
    #3 0x0000004ecce4 in std::_Function_handler<void (), fluid_dls_font::on_file_exit::{lambda()#1}>::_M_invoke(std::_Any_data const&) /usr/include/c++/15/bits/std_function.h:292
    #4 0x0000004e2ba3 in std::function<void ()>::operator()() const /usr/include/c++/15/bits/std_function.h:593
    #5 0x0000004e2ba3 in scope_guard<std::function<void ()> >::~scope_guard() /fluidsynth/src/sfloader/fluid_dls.cpp:142
    #6 0x0000004e2ba3 in fluid_dls_font::~fluid_dls_font() /fluidsynth/src/sfloader/fluid_dls.cpp:438
    #7 0x0000004e2ba3 in delete_fluid_dls_font /fluidsynth/src/sfloader/fluid_dls.cpp:523
    #8 0x0000004e2ba3 in fluid_dls_sfont_delete /fluidsynth/src/sfloader/fluid_dls.cpp:3150
    #9 0x00000043c066 in fluid_synth_sfunload_callback /fluidsynth/src/synth/fluid_synth.c:5692
    #10 0x00000041e299 in fluid_timer_run /fluidsynth/src/utils/fluid_sys.c:1002
    #11 0x7f0591f1e3ac  (/lib64/libglib-2.0.so.0+0x943ac) (BuildId: a1baf80bf47a8b0e99823b0b876b51fed88de515)
    #12 0x7f0592046c75  (/lib64/libasan.so.8+0x63c75) (BuildId: cbfe49f3b7600c4f194d4c54774c977296e9d98a)
    #13 0x7f0590d0edf0 in start_thread (/lib64/libc.so.6+0x9bdf0) (BuildId: 8523b213e7586a93ab00f6dd476418b1e521e62c)
    #14 0x7f0590d93a63 in __GI___clone (/lib64/libc.so.6+0x120a63) (BuildId: 8523b213e7586a93ab00f6dd476418b1e521e62c)

0x7b658f5ebda0 is located 32 bytes inside of 64-byte region [0x7b658f5ebd80,0x7b658f5ebdc0)
freed by thread T0 here:
    #0 0x7f05921038eb  (/lib64/libasan.so.8+0x1208eb) (BuildId: cbfe49f3b7600c4f194d4c54774c977296e9d98a)
    #1 0x0000004d60b2 in fluid_dls_loader_delete /fluidsynth/src/sfloader/fluid_dls.cpp:2985

previously allocated by thread T0 here:
    #0 0x7f0592104c2b in malloc (/lib64/libasan.so.8+0x121c2b) (BuildId: cbfe49f3b7600c4f194d4c54774c977296e9d98a)
    #1 0x00000057d249 in new_fluid_sfloader /fluidsynth/src/sfloader/fluid_sfont.c:102

Thread T5 created by T0 here:
    #0 0x7f05920fc192 in pthread_create (/lib64/libasan.so.8+0x119192) (BuildId: cbfe49f3b7600c4f194d4c54774c977296e9d98a)
    #1 0x7f0591f1d004  (/lib64/libglib-2.0.so.0+0x93004) (BuildId: a1baf80bf47a8b0e99823b0b876b51fed88de515)

SUMMARY: AddressSanitizer: heap-use-after-free /fluidsynth/src/sfloader/fluid_dls.cpp:387 in fluid_dls_font::on_file_exit::{lambda()#1}::operator()() const
Shadow bytes around the buggy address:
  0x7b658f5ebb00: fd fd fd fd fa fa fa fa fd fd fd fd fd fd fd fa
  0x7b658f5ebb80: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
  0x7b658f5ebc00: fd fd fd fd fd fd fd fa fa fa fa fa fd fd fd fd
  0x7b658f5ebc80: fd fd fd fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x7b658f5ebd00: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
=>0x7b658f5ebd80: fd fd fd fd[fd]fd fd fd fa fa fa fa fd fd fd fd
  0x7b658f5ebe00: fd fd fd fd fa fa fa fa fd fd fd fd fd fd fd fa
  0x7b658f5ebe80: fa fa fa fa fd fd fd fd fd fd fd fa fa fa fa fa
  0x7b658f5ebf00: fd fd fd fd fd fd fd fa fa fa fa fa fd fd fd fd
  0x7b658f5ebf80: fd fd fd fd fa fa fa fa fd fd fd fd fd fd fd fd
  0x7b658f5ec000: fa fa fa fa fd fd fd fd fd fd fd fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==398==ABORTING

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a heap-based use-after-free vulnerability where fluid_file_callbacks_t could be destroyed before all soundfonts using it were cleaned up. The fix ensures each soundfont maintains its own copy of the file callbacks structure rather than holding a pointer to a shared instance.

  • Changed fluid_file_callbacks_t from pointer to value in soundfont structures to prevent dangling pointer issues
  • Reordered destructor cleanup in delete_fluid_synth() to delete loaders after fonts have been unloaded
  • Updated all code accessing file callbacks to use direct member access (.) instead of pointer dereference (->)

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/synth/fluid_synth.c Moved SoundFont loader deletion to after lazy sfont unloading to prevent premature destruction of file callbacks
src/sfloader/fluid_dls.cpp Changed fcbs from pointer to value copy in fluid_dls_font struct and updated all access patterns accordingly
src/sfloader/fluid_defsfont.h Changed fcbs from pointer to value in _fluid_defsfont_t struct and added necessary include for fluid_sfont.h
src/sfloader/fluid_defsfont.c Updated to copy the fluid_file_callbacks_t struct by value and removed redundant include

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@derselbst derselbst merged commit 685e54c into master Dec 23, 2025
115 of 124 checks passed
@derselbst derselbst deleted the issue-1728 branch December 23, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Heap-based use after free during synth destruction

2 participants