microsoft / winfile

Original Windows File Manager (winfile) with enhancements
MIT License
6.75k stars 698 forks source link

Fix crash modifying file associations #439

Closed malxau-msft closed 1 month ago

malxau-msft commented 2 months ago

I encountered a crash trying to use the associations feature without being elevated:

  1. Add a new file type
  2. At the bottom of the dialog, associate the new type with an extension that is already associated with a different type
  3. Click ok. The registry updates fail.

What's happening:

The association logic has a "global" data structure tracking file types and extensions. When a new file type is being added, a new FILETYPE structure is allocated prior to opening the dialog (it is passed to AssociateFileDlgProc via DialogBoxParam.) Adding the extension in the child dialog (in AssociateFileDlgExtAdd) causes the "global" extension to be linked to the new file type. When clicking Ok, AssociateFileWrite calls FileTypeWrite to persist the new file type, then loops through extensions to update. Here, FileTypeWrite fails and frees the file type. The extensions still point to it, so when updating the extensions, it accesses freed memory.

Looking at the history, I believe this bug has existed for a very long time. The 16 bit version has a different (and simpler) file association model, but the 32 bit version has seen quite a few bugs and fixes in this area.

What this change is attempting to do is have FileTypeWrite perform the inverse of AssociateFileDlgExtAdd. It looks for any extensions linked to the new type, removes them, and if they were previously linked to a different type, links them with that. It also indicates that these extensions should not update the registry either, since in theory they have not changed.

I think there's a lot of scope to go further here. For one thing, it might be nice if a non-Administrator either has this UI disabled or is limited to update HKEY_CURRENT_USER\Software\Classes to eliminate the failure from happening. Personally I'd prefer that AssociateFileDlgProc doesn't attempt to manipulate the "global" structure until after it's successfully written to the registry.