Binary files getting labeled as missing DEP/ASLR/SIGN related flags

[m3vaz]

Describe the bug When running the analyzer on some .dll files, certain files are flagged as missing the following flags IMAGE_DLLCHARACTERISTICS_NX_COMPAT and IMAGE_DLLCHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLLCHARACTERISTICS_HIGH_ENTROPY_VA in the header.

Local debugging has it traced to GetDLLCharacteristics() which creates PeNet.FileParser.MMFile which uses MemoryMappedFile.CreateFromFile(). CreateFromFile() returns UnauthorizedAccessException on a read only file. GetDllCharacteristics() then handles the error, does a log print and returns an empty response, making the FileCollector think the dll is missing all charactersitics.

To Reproduce Steps to reproduce the behavior:

1. Run a file collector over an empty directory (Run 1)
2. Copy a write protected .dll file with NX_COMPAT/DYNAMIC_BASE/HIGH_ENTROPY_VA in the directory
3. Run a file collector over the same directory without write permissions to the file (Run 2)
4. Run analysis on Run 1/2 (analysis shows missing flags)
5. Add write permissions to the .dll file
6. Run a file collector with write permissions to the file (Run 3)
7. Run analysis on Run 1/3 (analysis does not show missing flags)

Expected behavior The analyzer should not need write access to the file it is analyzing. If the analyzer is not able to access a file, it should mark that as an error, it should not quietly dismiss the access error and then flag the file with different warnings.

Screenshots N/A

System Configuration (please complete the following information):

• OS: [e.g. Windows] Windows
• OS Version: [e.g. Windows 10 1803] Windows 10
• Application Version: [e.g. 2.0.1-beta+50af283871] 2.3.292
• CLI or GUI: both

Additional Context N/A

[gfs]

@m3vaz Thank you for the detailed report and reproduction instructions. I'll investigate and hopefully can get a fix out soon.

[gfs]

The analyzer should not need write access to the file it is analyzing.

I agree. It looks like PEFile has a ctor that takes a Stream, so as long as the file is readable we can retry to read it into a Stream and then perform the analysis from there.

If the analyzer is not able to access a file, it should mark that as an error, it should not quietly dismiss the access error

Is this a request to raise the verbosity of errors when fetching dll characteristics?

[m3vaz]

If the analyzer is not able to access a file, it should mark that as an error, it should not quietly dismiss the access error

Is this a request to raise the verbosity of errors when fetching dll characteristics?

I don't think increased verbosity would help, especially if that verbosity isn't reflected in the gui workflow. When using the gui, I would:

1. Scan with FileCollector
2. Change things on file system
3. Scan with FileCollector
4. Analyze
5. Export to json

---

The core issue is that the analysis (4.) reports incorrect information, when it should report that it doesn't have enough information to do the analysis on the specific file. The request would be that the access error is flagged in the scan (1. or 3.) and then reflected in the analysis (4.).

Right now the analysis will report:

        "Rules": [
          {
            "Description": "Flag when executables are created without DEP.",
            "Name": "Missing DEP"
          },
          {
            "Description": "Flag when executables are created without ASLR.",
            "Name": "Missing ASLR"
          },
          {
            "Description": "Flag when unsigned/incorrectly signed binaries are added.",
            "Name": "Unsigned binaries"
          }
        ]

instead it should report

        "Rules": [
          {
            "Description": "Flag when errors encountered during scanning",
            "Name": "Missing scan information"
          }

or something similar.

---

If modifying the analysis output is out of the question, then the alternative request would be to just stop the scan when an error is encountered.

[gfs]

I pushed a fix last night that should resolve reading signature and dllcharacteristic data from read only files on windows. #685

For the secondary requests, reporting gathering errors during analysis is a rather large change but I'll add a work item to consider it and think about how it could be done.

Exiting when encountering any collection error is an even larger change, and likely depends on the previous work to decide if it should exit.