Inconsistencies for import table parsing in real world data

[keithjjones]

Thanks for writing this analyzer!

I have installed this analyzer on 2 large networks. Through a custom package I added a column to the pe.log called "file_has_gaps" that will be set to T when the file_gap event fires. I also turned on extra logging with pe_log_import_table = T; to get the import table in the log.

When I compared the column has_import_table with the output of the import table in the logs, which signifies this event fired https://github.com/zeek/spicy-pe/blob/main/analyzer/analyzer.evt#L29, there are discrepancies. I will see hits for file_has_gaps == F and import table field does not exist and has_import_table == T which you would think should not happen unless the import table event is not firing correctly. It's not a handful of hits like this, it's closer to 10% of the PE files I am seeing (~300/3000) on these networks. I can't report the actual data, but I wanted to report the inconsistencies. Let me know if there is any other info I can provide. Thanks!

[keithjjones]

I checked the second network and it's closer to 15% with 66/421 of them having this quality.

[fox-ds]

Hi Keith, I spent some time yesterday trying to solve a similar issue. We also had some PCAP's containing malicious PE's, which weren't processed correctly. After I checked for the file_gap event, it turns out that also in our case, the file_gap event is raised for those PCAP's. I did some trial and error debugging and found that the analyzers stops processing the bytes at the

    sectionData:    bytes &size=self.headerPadSize + self.sectionDataSize
                        if ( self.fileSize <= MaxFileSize );

part, in which it tries to read all the bytes from the sections. If I change the &size to e.g. 100 it's able to fetch the first 100 bytes, If I change it to &eod, it just stops processing the bytes at a certain point (probably due to the missing data). This means that the subsequent code (parsing the Import & Export tables properly) isn't executed, leaving the import_table field empty.

However, the information whether an import/export table is available is stored in the PE Optional Header, which is parsed before the entire section bytes are read; that's probably why the has_import_table=T is set. Because the size of the headers is relatively small compared to the size of the combined sections, it's likely that the file misses some bytes in the sectionData and thus will skip parsing the import & export tables.

The question that arises is; is this behavior desired, or not? Should we add in an extra field by default that indicates whether a file_gap event occurred (like your custom package), or should we e.g. write a line to weird.log? Or should we somehow rewrite the code so that it just ignores the missing bytes and continues the parsing (not sure if this is actually possible though).

[keithjjones]

@fox-ds thanks for the quick response! In our case we are not seeing a file_gap event fire, yet the import directory is not parsed. That does not sound like your case? I understand that you have gaps in your pcaps?

We do see gaps (missing packets) on our live networks and it doesn't always parse the import directory, depending where the gaps are. In those cases, I see file_gap fire. I'm not concerned about these as much as I am the ones that do not have gaps but the import table is not parsed when the PE file says there is one. That category is the 10-15% of the missed import directories we are seeing, while the missed bytes cases have much lower numbers.

Thanks!

[fox-ds]

@keithjjones thanks for the clarification. We currently don't run the spicy-pe plugin with the additional file_gap field, so I can't give you any numbers on that yet. However, this is certainly something that we want to measure as well, so maybe we'll be able to come back to this in a while with some numbers.