Leor3961 / volatility

Automatically exported from code.google.com/p/volatility
0 stars 0 forks source link

Scanner framework loses the profile of any address space it's given to scan #26

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
So I am working on a plugin that analyzes a specific userland application and I 
hit a bug pretty nasty and non-obvious bug in the Scanner framework.

the problem is that I have created my own vtypes for the application in order 
to parse its structures and the way I find the "task" for the proces is either 
by the user-supplied PID or by searching by process name. Once I find the 
process, I then proceed to add my custom vtypes to the tasks address space so 
then I can process them as verinfo does.  this works perfect for everything but 
scanning.

The bug came when I went to scan the tasks address space, and anytime I tried 
to make a structure using the obj.Object method, the return value was always 
"None". After wasting an hour debugging the issue, I finally realized the issue 
was that my custom vtypes weren't being put into the buffer address space set 
as "self.address_space" in the inherited scan class, which means of course it 
knows nothing of my custom structure and so it couldn't instantiate it.

I fixed the bug for now by making my check function begin like this:

def check(self, offset):
     self.address_space.profile.add_types(my_types)

and then proceed to scan and its working now, but that was very non-obvious.

I believe this actually exposes two bugs, 

1) is that even when vol is run with -d, the obj.Object handler gave no error 
messages or warnings about why it couldn't instantiate the object

2) the scanning code needs to make the buffer address space object it uses 
inherit all the vtypes from the address space you pass it or ugly hacks like my 
fix will have to be used in all plugins that want to use custom vtypes with 
scanning

Original issue reported on code.google.com by atc...@gmail.com on 28 Aug 2010 at 8:00

GoogleCodeExporter commented 8 years ago
Thanks for the bug report, you're quite right about the second bug.  The buffer 
address space should have a copy of the "host" address space's profile, so that 
vtypes and so on are all carried over, I'll mock up a fix soon.

The problem with the first bug, is that there is no way of knowing when a 
NoneType is going to be wanted, or not.  The NoneType itself can contain a 
reason for being raised, but there is currently no mechanism for this to be 
revealed, except by hitting an exception or altering the code to drop into pdb 
or similar.

It's not clear how to resolve this since sometimes NoneObjects can be ignored, 
and sometimes they can't, and if there's several NoneObjects involved, knowing 
whether there's an issue and how to display it to the user is difficult.  As 
such, I think NoneObject debugging is left as an internal mechanism, which 
unfortunately requires developers to know it exists.

The best place to put documentation for this is on the wiki, so if you'd like 
to write something, please do and either submit it here, or to one of us, and 
we should also see about getting you access to write your own wiki pages.  If 
you don't want to write one, then please file another very short (one liner) 
bug asking for better documentation of it (and possibly also the scanner class 
as well).

Thanks again for tracking down the bug, I'm sorry it was so difficult to find.  
5:(

Original comment by mike.auty@gmail.com on 28 Aug 2010 at 11:47

GoogleCodeExporter commented 8 years ago
Ok, here's my proposed patch.  It takes the profile (and therefore vtypes, etc) 
from the address_space passed to the scan command and applies them to the 
buffer before running any checks against it.  Since the buffer should refer to 
the same object that the checks look at, these changes should propagate 
properly, but I'd like to test this before I commit it.  Could you please try 
the attached patch (removing your own manual fix for the problem) and let me 
know if that works/solves it for you please?

Original comment by mike.auty@gmail.com on 29 Aug 2010 at 12:02

Attachments:

GoogleCodeExporter commented 8 years ago
actually, it was my fault and it was inheriting properly. just close the bug as 
invalid, sorry about that.

Original comment by atc...@gmail.com on 29 Aug 2010 at 12:35

GoogleCodeExporter commented 8 years ago
Ok, well, it looks like the buffer, when created, will use the config.PROFILE, 
which could be carried around as global (so changes to it elsewhere might show 
up in the check's address_spaces).  Having said all that, it may still be 
worthwhile explicitly copying over the profile.  Did you manage to test this 
patch, and did it cause any problems?  If not, I might put it in anyway, just 
so we're clear as to what goes where.

Original comment by mike.auty@gmail.com on 29 Aug 2010 at 12:43

GoogleCodeExporter commented 8 years ago
It may be possible to just print the reason each time the NoneObject is 
instantiated when the debugging level is set to DEBUG.

As an aside we should probably start using the standard logging module for 
these kind of errors so that the debugging can be handled by more complex 
mechanisms (e.g. output to file, syslog etc).

Original comment by scude...@gmail.com on 30 Aug 2010 at 9:02

GoogleCodeExporter commented 8 years ago
Ok, I committed the volatility-scanner-profile.patch as r407 to make it more 
explicit in the code.  I've also added NoneObject instantiation debugging (as 
r408), although this requires "-dd" or "-d -d" since for certain plugins (such 
as files) it floods the output with attempted Object instantiations that are 
later intentionally ignored by the plugin.  I'm therefore going to mark this 
bug as Fixed.

The standard logging module support should just require a change to 
volatility/debug.py, but to get any of the more complex features (output to 
file/syslog etc) will need a bit more work, and so it should probably have it's 
own issue.

Original comment by mike.auty@gmail.com on 2 Sep 2010 at 11:57