Possible double-free and use-after-free in FileData::file_data_free_ci

[xsdg]

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.

The codepath for the issue

1. Here's FileData::file_data_free_ci
2. You can see that on line 1630, it calls FileData::file_data_planned_change_remove
3. 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
4. On line 730, file_data_free calls FileData::file_data_change_info_free
5. file_data_change_info_free calls g_free on fd->change->source, fd->change->dest, and fd->change before setting fd->change to nullptr.
6. Function control eventually returns back to FileData::file_data_free_ci
7. 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)
8. So when we attempt to dereference that pointer on line 1632, that is a use-after-free of fdci.
9. 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.
10. Then if we make it to lines 1634–1637, we repeat g_free on fdci->source, fdci->dest, and fdci. Each of those is a double-free.
11. 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.

[xsdg]

Just to record this here, I had created a feature request for the RAII object in #1415, and then implemented that object in #1420