This double-free relies on refcount state. If a FileData has two or more references when FileData::file_data_free_ci is called, then everything will be fine. However, if the FileData has 1 reference when the method is called, while holding a FileDataChangeInfo, a multiple memory errors will occur.
On line 1612, file_data_planned_change_remove calls ::file_data_unref on the FileData. If our refcount is 1, that will end up triggering file_data_free
First and foremost, you can see that file_data_free_ci had copied the fd->change pointer into a local called fdci before it called file_data_planned_change_remove. Thus, after we return from that function, fd->change == nullptr, but fdci retains the former value of that variable (which was already g_freed in step 5)
So when we attempt to dereference that pointer on line 1632, that is a use-after-free of fdci.
Also, because fd will have been g_freed at this point as well, passing fd to file_data_disable_grouping will also cause use-after-free errors in that function.
Then if we make it to lines 1634–1637, we repeatg_free on fdci->source, fdci->dest, and fdci. Each of those is a double-free.
Finally, on line 1639, we attempt fd->change = nullptr when fd has already been g_freed. This is a use-after-free
Possible mitigations
Because this function body relies on the FileData continuing to exist, the easiest mitigation would be to file_data_ref the fd before the function starts, and to file_data_unref the fd before returning (which may trigger the fd to be freed). Then, we should re-cache fd->change after file_data_planned_change_remove returns, and re-check whether it might be null.
Because there are multiple return points, this would be a lot easier with an RAII FileDataRef object.
This double-free relies on refcount state. If a
FileData
has two or more references whenFileData::file_data_free_ci
is called, then everything will be fine. However, if theFileData
has 1 reference when the method is called, while holding aFileDataChangeInfo
, a multiple memory errors will occur.The codepath for the issue
FileData::file_data_free_ci
FileData::file_data_planned_change_remove
file_data_planned_change_remove
calls::file_data_unref
on theFileData
. If our refcount is 1, that will end up triggeringfile_data_free
file_data_free
callsFileData::file_data_change_info_free
file_data_change_info_free
callsg_free
onfd->change->source
,fd->change->dest
, andfd->change
before settingfd->change
tonullptr
.FileData::file_data_free_ci
file_data_free_ci
had copied thefd->change
pointer into a local calledfdci
before it calledfile_data_planned_change_remove
. Thus, after we return from that function,fd->change == nullptr
, butfdci
retains the former value of that variable (which was alreadyg_free
d in step 5)fdci
.fd
will have beeng_free
d at this point as well, passingfd
tofile_data_disable_grouping
will also cause use-after-free errors in that function.g_free
onfdci->source
,fdci->dest
, andfdci
. Each of those is a double-free.fd->change = nullptr
whenfd
has already beeng_free
d. This is a use-after-freePossible mitigations
Because this function body relies on the
FileData
continuing to exist, the easiest mitigation would be tofile_data_ref
thefd
before the function starts, and tofile_data_unref
thefd
before returning (which may trigger thefd
to be freed). Then, we should re-cachefd->change
afterfile_data_planned_change_remove
returns, and re-check whether it might be null.Because there are multiple return points, this would be a lot easier with an RAII
FileDataRef
object.