volatility needs more stringent argument checking

[GoogleCodeExporter]

I was looking at the commandline from the screenshot from issue 278 and noticed 
that Volatility ran in spite of the wrong commandline:

$ volatility -profile=WinSP3x86 -f stuxnet.vmem malfind -D ./teststux

the issue is: "-profile=WinSP3x86" instead of "--profile=WinXPSP3x86"

In this case -profile becomes -p and self._config.PID becomes "rofile=WinSP3x86"

we should print out an error when -p is passed non ints instead of silently 
ignoring.

http://code.google.com/p/volatility/source/browse/trunk/volatility/plugins/taskm
ods.py#81

Original issue reported on code.google.com by jamie.l...@gmail.com on 28 Jun 2012 at 4:01

[GoogleCodeExporter]

from attrc, an accident:

python vol.py --profile=LinuxOpenSuSE12x86 linux_lsmod

Volatile Systems Volatility Framework 2.2_alpha
No suitable address space mapping found
Tried to open image as:
 LimeAddressSpace: lime: need base
 WindowsHiberFileSpace32: No base Address Space
 WindowsCrashDumpSpace64: No base Address Space
 WindowsCrashDumpSpace32: No base Address Space
 AMD64PagedMemory: No base Address Space
 JKIA32PagedMemory: No base Address Space
 JKIA32PagedMemoryPae: No base Address Space
 IA32PagedMemoryPae: Module disabled
 IA32PagedMemory: Module disabled
 FileAddressSpace - EXCEPTION: 'NoneType' object has no attribute 'startswith'
and it gave that weird error instea dof saying I didnt specifiy -f

should that be a better warning?

Original comment by michael.hale@gmail.com on 8 Sep 2012 at 5:23

[GoogleCodeExporter]

So that second issue was caused by AbstractLinuxCommand trying to load an AS in 
the initializer, rather than once execute method's been called.  I've applied 
r2406 to solve this, and I've moved profile to a property so that it stays in 
sync with the address space.  Please let me know of any problems...

Original comment by mike.auty@gmail.com on 8 Sep 2012 at 7:53

[GoogleCodeExporter]

It does break... when a plugin now tries to get a symbol, this happens:

  File "/root/ikelos-patch/volatility/plugins/linux/lsmod.py", line 140, in calculate
    modules_addr = self.get_profile_symbol("modules")
  File "/root/ikelos-patch/volatility/plugins/linux/common.py", line 72, in get_profile_symbol
    return self.profile.get_symbol(sym_name, nm_type, sym_type, module)
  File "/root/ikelos-patch/volatility/plugins/linux/common.py", line 48, in profile
    return self.addr_space.profile
AttributeError: 'NoneType' object has no attribute 'profile'

so it seems its not getting the version that is set in execute

Original comment by atc...@gmail.com on 8 Sep 2012 at 8:41

[GoogleCodeExporter]

Can you please include the complete backtrace, calculate should only have been 
called from execute which should have instantiated self.addr_space, but I can't 
see whether it has or not from the output you provided...

Original comment by mike.auty@gmail.com on 8 Sep 2012 at 8:49

[GoogleCodeExporter]

So I'm having difficulty recreating this locally.  Could you please check if 
this happens on a clean trunk download, and if so include the full backtrace?

Original comment by mike.auty@gmail.com on 8 Sep 2012 at 9:06

[GoogleCodeExporter]

It seems to be plugins that call calculate on other plugins (e.g psxview)

python vol.py --profile=LinuxCentOS63x64 -f 
/root/cent-second/centos-6.3-x86_64-LiveDVD-avgcoder.mem linux_psxview
Volatile Systems Volatility Framework 2.2_alpha
Offset(V)          Name                    PID pslist pid_hash kmem_cache
------------------ -------------------- ------ ------ -------- ----------
Traceback (most recent call last):
  File "vol.py", line 185, in <module>
    main()
  File "vol.py", line 176, in main
    command.execute()
  File "/root/ikelos-patch/volatility/plugins/linux/common.py", line 52, in execute
    commands.Command.execute(self, *args, **kwargs)
  File "/root/ikelos-patch/volatility/commands.py", line 111, in execute
    func(outfd, data)
  File "/root/ikelos-patch/volatility/plugins/linux/psxview.py", line 84, in render_text
    for offset, process, ps_sources in data:
  File "/root/ikelos-patch/volatility/plugins/linux/psxview.py", line 55, in calculate
    ps_sources['pslist']     = self.get_pslist()
  File "/root/ikelos-patch/volatility/plugins/linux/psxview.py", line 40, in get_pslist
    return [x.obj_offset for x in linux_pslist.linux_pslist(self._config).calculate()]
  File "/root/ikelos-patch/volatility/plugins/linux/pslist.py", line 39, in calculate
    init_task_addr = self.get_profile_symbol("init_task")
  File "/root/ikelos-patch/volatility/plugins/linux/common.py", line 71, in get_profile_symbol
    return self.profile.get_symbol(sym_name, nm_type, sym_type, module)
  File "/root/ikelos-patch/volatility/plugins/linux/common.py", line 48, in profile
    return self.addr_space.profile
AttributeError: 'NoneType' object has no attribute 'profile'

----

I guess these plugins could be "wrong" but I am pretty sure that is the same 
way the windows psxview works.

Original comment by atc...@gmail.com on 8 Sep 2012 at 9:20

[GoogleCodeExporter]

Ok, I think I've found it.  It seems that some linux plugins instantiate whole 
other plugins to do their dirty work, and instead of calling execute, just call 
the calculate function.  That makes sense, but is quite unfortunate because it 
means that every linux plugin will then need to instantiate it's own address 
space on initialization.

This is one of the reasons we used to use inheritance until it was classed as 
too confusing.  In this case, the netstat plugin instantiates the lsof plugin 
but that no longer loads a second copy of the address space (which is sensible 
because address space loading is time consuming).  Depending on how often this 
is done, one option is to set the target plugin's address space to be the one 
we've already got, or we can go back to instantiating an address space for each 
plugin we load which will slow down the start of every volatility run 
regardless of whether it's a linux or windows plugin we're running.  I'd really 
vote for the first if it's at all possible...

Original comment by mike.auty@gmail.com on 8 Sep 2012 at 9:23

[GoogleCodeExporter]

The windows psxview calls tasks.pslist and passes it the address space, so 
that's not instantiating a new plugin and running that fresh.  This may need a 
bit of thinking about and probably won't get solved tonight, but if you could 
be looking for places in the linux code where you instantiate a new plugin to 
call its calculate function, that would help us see which option will be best 
to take...

Original comment by mike.auty@gmail.com on 8 Sep 2012 at 9:26

[GoogleCodeExporter]

I see what it does with all_tasks, but I meant more like this one:

http://code.google.com/p/volatility/source/browse/trunk/volatility/plugins/malwa
re/psxview.py#82

def check_psscan(self):
        """Enumerate processes with pool tag scanning"""
        return dict((p.obj_offset, p)
                    for p in filescan.PSScan(self._config).calculate())

That is what I was modeling my psxview off. If you can help me figure out how 
the filescan.PSSCan call is different from the ones I do then I can better help 
fix it.

Original comment by atc...@gmail.com on 8 Sep 2012 at 9:31

[GoogleCodeExporter]

Sure, you're cheating by assuming the AS is already set when calculate gets 
called whereas all the windows plugins stand alone and calculate it for 
themselves when they need it (ie at the start of every calculate call). 
Unfortunately that'd be a change that should be applied to every Linux 
plugin... 5:(

Original comment by mike.auty@gmail.com on 8 Sep 2012 at 10:11

[GoogleCodeExporter]

alright. I am just going to make the change so it gets set in all the plugins. 
I did not know about the execute stuff when I first make AbstractLinuxCommand

Original comment by atc...@gmail.com on 9 Sep 2012 at 5:59

[GoogleCodeExporter]

done - http://code.google.com/p/volatility/source/detail?r=2431

Original comment by atc...@gmail.com on 9 Sep 2012 at 6:20

[GoogleCodeExporter]

Cool, looks good, thanks for going through all the code and fixing that, sorry 
it wasn't clearer from the get-go...

Original comment by mike.auty@gmail.com on 9 Sep 2012 at 10:07

[GoogleCodeExporter]

Guys, how does this patch look for addressing the original issue (when -p/--pid 
is passed strings rather than integers)? 

I couldn't immediately tell why we were catching TypeError, so its removed in 
the patch...but maybe someone (maybe Ikelos) can help me understand why and 
then we'll add it back. There's the lines:

# Make this a separate statement, so that if an exception occurs, no harm done
tasks = newtasks

Is that what has raised TypeError in the past? Just wondering...thanks.

Original comment by michael.hale@gmail.com on 4 Jan 2013 at 4:30

Attachments:

• improve_pid_argument_check_issue279.patch

[GoogleCodeExporter]

Hiya,

So in general it was expected to be handed non-integers so that it could deal 
with multiple PIDs.  So:

-p 513  (_config.PID = 513)
-p 513,514 (_config.PID = "513,514")

We could look at allowing multiple -p flags, but it would be a break from 
current usage, so not one I'd suggest until volatility 3 is ready...

Since we return tasks, we try not to set it until we know whether an exception 
will have happened (ie, one of the tasks we'd found didn't have a 
UniqueProcesssID property) in which case tasks will maintain its previous value 
of all processes (causing a hopefully graceful fallback).  whether tasks would 
actually get set or not in real life is another question, but having the 
UniqueProcessID check outside of the exception handling will almost certainly 
cause exceptions to be seen that hadn't been previously...

Original comment by mike.auty@gmail.com on 4 Jan 2013 at 9:15

[GoogleCodeExporter]

Hey Mike, 

Thanks for the reply! Yeah, entering -p 513 or -p 513,514 is fine both with and 
without the patch. What we're trying to prevent with the patch is silent 
strange behavior when someone enters -p foo or -p "foo" 

The original issue was someone entered -profile=WinXPSP3x86 instead of 
--profile=WinXPSP3x86 causing our argument parser to treat it like -p 
"rofile=WinSP3x86" and then in filter_tasks it did int("rofile=WinSP3x86") 
which caused a ValueError. 

Hmm, is it possible that a task wouldn't have a UniqueProcessId property? Even 
if it didn't, wouldn't that be AttributeError and not TypeError? I can't think 
of a situation where it wouldn't have a UniqueProcessId. 

Thanks!

Original comment by michael.hale@gmail.com on 4 Jan 2013 at 3:34

[GoogleCodeExporter]

Is there any point in discussing this issue further when it is already
solved in the tech preview branch?  Both these forms are supported as well
as providing a list of pids. For example

-p 23 26 662

-p 23,26,662

Original comment by scude...@gmail.com on 4 Jan 2013 at 9:24

[GoogleCodeExporter]

Hiya, you're quite right, sorry, it would be a ValueError in that instance.  I 
also misread the patch and didn't think it handled string PIDs, so probably the 
patch is fine (that'll teach me to answer questions just after I've woken up)...

Original comment by mike.auty@gmail.com on 5 Jan 2013 at 5:05

[GoogleCodeExporter]

This issue was closed by revision r2839.

Original comment by michael.hale@gmail.com on 5 Jan 2013 at 8:12

• Changed state: Fixed

[GoogleCodeExporter]

Thanks for clarifying Mike. I applied the patch and that should fix things up. 

Original comment by michael.hale@gmail.com on 5 Jan 2013 at 8:13