microsoft / AttackSurfaceAnalyzer

Attack Surface Analyzer can help you analyze your operating system's security configuration for changes during software installation.
MIT License
2.68k stars 271 forks source link

In Use files are getting labeled as missing DEP/ASLR/SIGN related flags #696

Closed jbuettner00 closed 10 months ago

jbuettner00 commented 10 months ago

When running the analyzer on files that are in use (.dll, .bpl, .exe), these files are being flagged as missing the DEP, ASLR, and signature in the report. Upon running the collector with the debug options we found that a System.IO.IOException is being returned for these files so no data is being collected. These files are then flagged in the analysis as missing DEP and ASLR and having Unsigned binaries. Issue #684 and #685 seems to have fixed this for only UnauthorizedAccessException.

To Reproduce Steps to reproduce the behavior:

  1. Run a file collector over an empty directory (Run 1)
  2. Install the app in question and then launch that app so it's files are in use
  3. Run a file collector over the directory of the newly installed app (Run 2)
  4. Run analysis on Run 1/2 (analysis shows missing flags)
  5. Exit the running app
  6. Run the file collector over the same directory of the app (Run 3)
  7. Run analysis on Run 1/3 (analysis does not show missing flags)

Expected behavior The analyzer should try to open a file in read only for any exception that is returned when the file is attempted to be opened in read/write. Also if there is still an exception after trying to get the file data in read only then the files should not be falsely flagged as missing the flags, instead there should be a "No Data" warning or something similar. This is similar to what is logged in issue #684

Screenshots image image

System Configuration (please complete the following information):

Additional Context We edited the WindowsFileSystemUtils.cs to make it more generic so it tries to open a file in read only for any exception instead of just for UnauthorizedAccessException. We also updated public static Signature? GetSignatureStatus(string Path) with the same change then running the analyzer on the in use files does not flag them as missing the DEP\ASLR\SIGN flags. Attached is the updated WindowsFileSystemUtils.cs. WindowsFileSystemUtils.cs.txt

gfs commented 10 months ago

Thanks for the report and proposed fix.

gfs commented 10 months ago

I went with a slightly less drastic change to add IOException specifically to the retry logic in #698. If you know are aware of other exceptions that can be thrown here that are not fatal, we can add those as well to the retry block. Some of the other exceptions that are possible are truly fatal and not worth retrying (NullReference etc).

I also opened a feature request for your other comment about tracking exceptions to distinguish better between failure to gather data and true detection of fields in particular for boolean fields. #699.

jbuettner00 commented 10 months ago

Awesome, thanks!

gfs commented 10 months ago

I believe this will be resolved with merging #698, but let me know if you encounter any other issues here.

Thanks, Gabe

jbuettner00 commented 10 months ago

Thanks, will do.