Fix symbol table PDB API that mixes returns, None values, and exceptions #1696
+6
−4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…turning an empty symbol_table name when the PDB cannot be downloaded.
atcuno
changed the title
…turning an empty symbol_table name when the PDB cannot be downloaded.
ikelos
reviewed
Mar 9, 2025
| @@ -409,6 +409,12 @@ def symbol_table_from_pdb( | |||
| _, symbol_table_name = cls._modtable_from_pdb( | |||
| context, config_path, layer_name, pdb_name, module_offset, module_size | |||
| ) | |||
|
|
|||
| if symbol_table_name is None: | |||
| raise exceptions.VolatilityException( | |||
There was a problem hiding this comment.
Should probably be a SymbolSpaceError or something more specific, but otherwise looks good.
There was a problem hiding this comment.
That would require changing the modtable_from_pdb and also the current callers. Is that worth it?
There was a problem hiding this comment.
Why would it? SymbolSpaceError inherits from VolatilityException, but people that want to catch only SymbolSpaceErrors can?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@ikelos this is that weird API I explained to you in DM on Slack, where you can ask for a symbol table then, if the PDB string is found in memory, but the symbol server doesn't host the GUID, you run into problems.
In particular, the function usually throws the VolatilityException whenever a PDB downloading/parsing issue happens, but the code paths traversed when finding a GUID, but the symbol server not having it, were returning None.
The external API for this (
symbol_table_from_pdb) did not correctly handle this case, which led to backtraces in plugins that were doing try/catch over the call thinking that would cover all error conditions and that the returned string name of the module would be set.We recently patched a fix into netstat so it can run in the test harness as this is what the backtrace would look like otherwise when run on our internal symbol server that did not have the particular tcpip.sys PDB. The None was coming from the
symbol_table_from_pdbcall. We can now take this extra check out since the API handles it and the code is already doing try/except as expected.