SanityCheckException, --unsafe, refactor of PE dumping plugins

GoogleCodeExporter commented 9 years ago

This seems like a valid question, so worth a little pro/con discussion. Is the 
--unsafe parameter needed for pe dumping plugins or should we *always* try 
hardest and just report sanity check and invalid format exception messages to 
the user? 

http://code.google.com/p/volatility/issues/detail?id=243#c5

Original issue reported on code.google.com by michael.hale@gmail.com on 7 Jul 2012 at 8:29

GoogleCodeExporter commented 9 years ago

"Im a little confused about this new SanityCheckException exception. Its 
inconsistent with the NoneObject design pattern. In most other cases we return 
a NoneObject in case of errors, but here we raise an exception.

The original purpose of the NoneObject design was to avoid having to catch 
exceptions all the time just like the code above:

try:
    dos_header = obj.Object("_IMAGE_DOS_HEADER", offset = self.DllBase,
                            vm = self.obj_native_vm)
    return dos_header.get_nt_header()
...
except exceptions.SanityCheckException:
   return obj.NoneObject("Failed initial sanity checks. Try -u or --unsafe")

Are we changing the NoneObject design in favour of exceptions now?"

Original comment by michael.hale@gmail.com on 8 Jul 2012 at 3:23

Changed title: Is --unsafe option needed (Confusion over SanityCheckException)
Added labels: Priority-Low
Removed labels: Priority-Medium

GoogleCodeExporter commented 9 years ago

Thanks Michael,
  I was only asking about this exception because reading the code here:

http://code.google.com/p/volatility/source/browse/trunk/volatility/plugins/overl
ays/windows/windows.py#792

It looks like the exception is raised from sanity_check_section() which is 
called from get_sections(). This means that if there are say 4 sections, and 
section 3 is invalid, then whoever calls get_sections() will process 2 
sections, and then raise the exception, losing the 4th section. (get_sections 
is currently a generator).

This is kind of the same problem we had with the NoneObject - say you try to 
instantiate a String() somewhere or dereference a pointer, and it fails - an 
exception will break the flow of processing and it becomes clumsy to handle the 
exception at each level. This is why I was specifically asking if the 
NoneObject design pattern was deliberately forgone in this case - and indeed if 
there are other cases where it should be.

So I guess the main decision is really whether we should return a NoneObject() 
for the broken section, or raise. The first method returns all the sections 
which are still sane. The second method results in an inconsistent output - 
some sections (before the error) are still processed, but later sections are 
not. If we want to abort processing altogether maybe we should change 
get_sections() to return a list rather than a generator, so the exception kills 
all processing (either an all or nothing solution).

I can also see see one place in idt.py where get_sections() is called with 
False for the unsafe parameter - if the user receives an exception here, there 
is not a lot they can do (i.e. the advice about the extra flag is not right and 
might confuse).

Original comment by scude...@gmail.com on 8 Jul 2012 at 5:02

GoogleCodeExporter commented 9 years ago

I'd guess the ValueError in sanity_check_section() was originally to capture 
user attention and make them acknowledge a given PE's section appeared 
malformed. After seing the reason, one could make a decision to ignore it and 
try dumping anyway. This does imply that the plugin tries and then requires 
permission to try harder (which is strange because we should always try 
hardest), but it also prevents the plugin from behaving badly without warning. 
For example, an attacker modifies 10 DLLs in 10 different processes to each 
show a section whose VirtualSize == 0xFFFFFFFF (4 GB). If the analyst launches 
dlldump, it wouldn't finish until 400+ GB of disk was taken. In this scenario I 
might prefer my tool to:

* Only dump valid/safe looking DLLs, but notify me of the 10 that seem malformed
* Give me an option to still extract the 10 malformed

So that's the nature of the --unsafe option and exceptions *I believe* (their 
implementation was before my time). 

What I might propose is first moving vaddump, dlldump, procexedump, and moddump 
to using table_header/table_row. The last column can state the result of the 
operation - which would be the output file name if dumping succeeded, or an 
error message (PEB not memory resident, ImageBase paged, section sanity check 
failed, no MZ or PE signature, etc). 

For example:

$ python vol.py -f PhysMem.bin --profile=Win2003SP2x86 procexedump -D out
Volatile Systems Volatility Framework 2.1_alpha
Process    ImageBase  Name                 Result
---------- ---------- -------------------- ------
0x8a39a648 ---------- System               Error: PEB is not memory resident
0x891f7c78 0x48580000 smss.exe             OK: executable.344.exe
0x8a1b2210 0x4a680000 csrss.exe            OK: executable.400.exe
0x8a14cd88 0x01000000 winlogon.exe         OK: executable.424.exe
0x8a14a2b0 0x01000000 services.exe         OK: executable.472.exe
0x8a10b420 0x01000000 lsass.exe            OK: executable.484.exe
0x891c8290 0x01000000 svchost.exe          OK: executable.668.exe
0x89ec1868 0x01000000 svchost.exe          OK: executable.792.exe
0x89e93ce8 0x01000000 svchost.exe          OK: executable.856.exe
0x89ecfbc8 0x01000000 svchost.exe          OK: executable.892.exe
0x8a154940 0x01000000 svchost.exe          OK: executable.908.exe
0x89fe3570 0x00400000 msdtc.exe            Error: ImageBaseAddress is not 
memory resident
0x89fb95e8 0x00400000 hpacubin.exe         Error: ImageBaseAddress is not 
memory resident
[snip]

I don't mind creating a patch for those 4 plugins to make them appear like the 
above. If we then decided to replace the exceptions with NoneObjects, we'd have 
a place to communicate the NoneObject reason to the user. We'd still to make 
some decisions about which cases to automatically try hardest or which ones 
need permission (if any). But hopefully this would at least be a start to some 
enhancements with the PE dumping plugins?

Original comment by michael.hale@gmail.com on 8 Jul 2012 at 8:38

GoogleCodeExporter commented 9 years ago

Michael,
  Yes that seems reasonable. You are correct in saying that a malformed section header can cause crazy large images to be written. I might suggest that there should probably be an ultimate condition over the total image size - i.e. something like --max_size or similar. This will ensure that images larger than say 100mb (which are legitimately extremely uncommon) wont be written, but images smaller than that will, despite potentially failing to validate.

There are other types of corruption - for example overlapping sections (which 
are currently handled by a different module altogether - procmemdump - should 
be expanded to all other pe dumping plugins), but these can be left for the 
next release.

Original comment by scude...@gmail.com on 8 Jul 2012 at 9:27

GoogleCodeExporter commented 9 years ago

Here's the plan that I have put together so far. Its just a draft, so if you 
have other points or just opinions in the matter, feel free to update/discuss. 
Also, I attached an example patch for accomplishing goals 1-3 (all three goals 
were combined into one patch due to how they work together). However, the next 
patch, for goal 4 would be separate. Finally, after a bit more analysis on 
which PE dumping cases we should handle and how, we can cut a patch for goal 5. 

1. Increase code that can be shared between dlldump, moddump, procexedump, 
vaddump
2. Convert output to table renderers, result (dumped file name or error 
message) in far right column
3. Construct more meaningful error messages for the cases in which pe dumping 
fails 
4. Use debug.warning for 'Memory not accessible' messages in procdump.py. this 
way we won't need to pass 
outfd to ProcExeDump.get_image() which is currently kind of awkward. Code in a 
plugin's 
calculate function should be able to rebuild a PE, but get_image() requires an 
outfd, 
and outfd isn't typically available in calculate. This would also, down the 
road, make it easier to 
move ProcExeDump.get_image() to an object method like 
_IMAGE_DOS_HEADER.get_image()
5. Document the most common errors and what we should do when they occur (raise 
or NoneObject)?
  a) Cannot acquire an address space
    * Action: Fail (or implement a backup). Report reason to user
    * Backup: AW's dumpfile API might work w/ kernel AS?
  b) PEB and/or ImageBase is paged (process only)
    * Action: Fail (or implement a backup). Report reason to user
    * Backup: Any method of finding the main EXE's base address that does not go through PEB. Hmm?
  c) PE sections sanity check failed
    * Do we raise or return NoneObject?
    * If one section is not valid, but others before and after it are, what should we do?
    * Instead of --unsafe, should we set a maximum PE section size, so we can always try the hardest 
      without explicit permission...as long as we can still convey notices to the user? 
  d) PE sections overlap
  e) Missing DOS signature at offset 0 (MZ)
    * Fail. Tell user its still possible to extract the containing memory region with vaddump
  f) Missing NT signature at elfanew (PE)
    * Fail. Tell user its still possible to extract the containing memory region with vaddump

So after the first patch, the plugins will just be sharing a bunch of code 
instead of duplicating, and they'll be printing output in the table format. 

Here's an example of dlldump before:

$ python vol.py -f b0f427db.vmem --profile=Win7SP0x86 -D out/ dlldump
Volatile Systems Volatility Framework 2.1_alpha
Dumping smss.exe, Process: smss.exe, Base: 48230000 output: 
module.276.3e515020.48230000.dll
Cannot dump smss.exe@ntdll.dll at 77b70000
Dumping csrss.exe, Process: csrss.exe, Base: 4a5a0000 output: 
module.372.3dafc268.4a5a0000.dll
Cannot dump csrss.exe@ntdll.dll at 77b70000
Cannot dump csrss.exe@USER32.dll at 763d0000
Cannot dump csrss.exe@CSRSRV.dll at 75d60000

And dlldump after:

$ python vol.py -f b0f427db.vmem --profile=Win7SP0x86 -D out/ dlldump 
Volatile Systems Volatility Framework 2.1_rc1
Process(V) Name                 Module Base Module Name          Result
---------- -------------------- ----------- -------------------- ------
0x85b15020 smss.exe             0x048230000 smss.exe             OK: 
module.276.3e515020.48230000.dll
0x85b15020 smss.exe             0x077b70000 ntdll.dll            Error: DllBase 
is paged
0x864fc268 csrss.exe            0x04a5a0000 csrss.exe            OK: 
module.372.3dafc268.4a5a0000.dll
0x864fc268 csrss.exe            0x077b70000 ntdll.dll            Error: DllBase 
is paged
0x864fc268 csrss.exe            0x0763d0000 USER32.dll           Error: DllBase 
is paged
0x864fc268 csrss.exe            0x075d60000 CSRSRV.dll           Error: DllBase 
is paged

Moddump before:

$ python vol.py -f b0f427db.vmem --profile=Win7SP0x86 -D out/ moddump
Volatile Systems Volatility Framework 2.1_alpha
Dumping ntoskrnl.exe, Base: 82840000 output: driver.82840000.sys
Dumping hal.dll, Base: 82809000 output: driver.82809000.sys
Dumping Npfs.SYS, Base: 8bed9000 output: driver.8bed9000.sys
Dumping VIDEOPRT.SYS, Base: 8be88000 output: driver.8be88000.sys
Dumping NETIO.SYS, Base: 86f3a000 output: driver.86f3a000.sys
Dumping vga.sys, Base: 8be7c000 output: driver.8be7c000.sys
Dumping fltmgr.sys, Base: 86c5d000 output: driver.86c5d000.sys
Dumping rdpencdd.sys, Base: 8bebe000 output: driver.8bebe000.sys
Cannot dump cdd.dll at 92c00000
Dumping spldr.sys, Base: 87000000 output: driver.87000000.sys

Moddump after:

$ python vol.py -f b0f427db.vmem --profile=Win7SP0x86 -D out/ moddump
Volatile Systems Volatility Framework 2.1_rc1
Module Base Module Name          Result
----------- -------------------- ------
0x082840000 ntoskrnl.exe         OK: driver.82840000.sys
0x082809000 hal.dll              OK: driver.82809000.sys
0x08bed9000 Npfs.SYS             OK: driver.8bed9000.sys
0x08be88000 VIDEOPRT.SYS         OK: driver.8be88000.sys
0x086f3a000 NETIO.SYS            OK: driver.86f3a000.sys
0x08be7c000 vga.sys              OK: driver.8be7c000.sys
0x086c5d000 fltmgr.sys           OK: driver.86c5d000.sys
0x08bebe000 rdpencdd.sys         OK: driver.8bebe000.sys
0x092c00000 cdd.dll              Error: Cannot acquire AS
0x087000000 spldr.sys            OK: driver.87000000.sys

Procexedump before:

$ python vol.py -f b0f427db.vmem --profile=Win7SP0x86 -D out/ procexedump
Volatile Systems Volatility Framework 2.1_alpha
************************************************************************
Error: PEB not memory resident for process [4]
************************************************************************
Dumping smss.exe, pid:    276 output: executable.276.exe
************************************************************************
Dumping csrss.exe, pid:    372 output: executable.372.exe
************************************************************************
Dumping wininit.exe, pid:    424 output: executable.424.exe
************************************************************************
Dumping csrss.exe, pid:    432 output: executable.432.exe
************************************************************************

Procexedump after:

$ python vol.py -f b0f427db.vmem --profile=Win7SP0x86 -D out/ procexedump
Volatile Systems Volatility Framework 2.1_rc1
Process(V) ImageBase  Name                 Result
---------- ---------- -------------------- ------
0x84138c78 ---------- System               Error: PEB at 0x0 is paged
0x85b15020 0x48230000 smss.exe             OK: executable.276.exe
0x864fc268 0x4a5a0000 csrss.exe            OK: executable.372.exe
0x8676f530 0x00f90000 wininit.exe          OK: executable.424.exe
0x85264a58 0x4a5a0000 csrss.exe            OK: executable.432.exe
0x869efd40 0x00060000 vmware-usbarbi       Error: ImageBaseAddress at 0x60000 
is paged
0x855c97d8 0x00400000 ollydbg.exe          OK: executable.4088.exe
0x84aeec18 0x00230000 sigverif.exe         Error: VirtualSize ffffffff is 
larger than image size.
0x852ee030 0x000d0000 audiodg.exe          OK: executable.5364.exe
0x84bed1b0 ---------- cmd.exe              Error: PEB at 0x7ffd4000 is paged

Original comment by michael.hale@gmail.com on 9 Jul 2012 at 1:39

Attachments:

issue_291_goals_123.patch

GoogleCodeExporter commented 9 years ago

I'm updating the description of this issue and attaching a slightly new patch 
for goals 1, 2, 3. 

Also, I'm going to ask for opinions on applying this patch for 2.1. 

Cons:
  - its past the RC1 date 

Pros:
  - the patch doesn't introduce new features or change any APIs
  - it helps one of the existing features in 2.1 (table rendering) become more complete 
  - its relatively small, thus we can easily/quickly make sure it cannot negatively impact existing plugins

Original comment by michael.hale@gmail.com on 10 Jul 2012 at 2:33

Changed title: SanityCheckException, --unsafe, refactor of PE dumping plugins

Attachments:

issue_291_goals_123.patch

GoogleCodeExporter commented 9 years ago

Michael, 
  The patch looks good to me. Some comments:

1) Use "with" for autoclosing the file - its more cleaner (we have a minimum 
requirement of 2.6 now so we can use with):

with open(os.path.join(self._config.DUMP_DIR, dump_file), 'wb') as fd:
   ....

2) for offset, code in self.get_image(outfd, space, base):
          of.seek(offset)
          of.write(code)

Is a bit more concise.

3) I am not a big fan of else clauses in exceptions - I would just put the 
result = "OK: {0}".format(dump_file) after the for loop.

I agree with your assessment re rc1.

Original comment by scude...@gmail.com on 10 Jul 2012 at 2:45

GoogleCodeExporter commented 9 years ago

Awesome, thanks for the comments. I attached a new patch which has been 
modified for accepting #2 and #3. I don't have any particular problem with #1, 
but if we make the move to use "with" syntax, I'd like to make it to all 
files/plugins at the same time (so that can be done separately). I do like that 
it autocloses the file handle, that is quite convenient. 

By the way, I actually didn't know the min requirement is 2.6. What other 
components require 2.6 (just so I know when updating the FAQ and Install Guide 
wiki pages)?

Original comment by michael.hale@gmail.com on 10 Jul 2012 at 3:01

Attachments:

issue_291_goals_123.patch2

GoogleCodeExporter commented 9 years ago

According to http://code.google.com/p/volatility/wiki/FAQ 2.6 is the minimum 
requirement.

Original comment by scude...@gmail.com on 10 Jul 2012 at 3:03

GoogleCodeExporter commented 9 years ago

LOL <embarrassed> OK I will read my own FAQ from now on ;-)

Original comment by michael.hale@gmail.com on 10 Jul 2012 at 3:12

GoogleCodeExporter commented 9 years ago

Issue 161 has been merged into this issue.

Original comment by michael.hale@gmail.com on 9 Aug 2012 at 4:42

GoogleCodeExporter commented 9 years ago

Programmatically handling and safely recovering from all of the possible ways a 
PE header can be corrupted is not really something we need to deal with at the 
moment. If in process space and the procdump/dlldump plugins fail due to 
corruption, we can always fall back on dumping the VAD. If in kernel space, we 
can carve the memory range with volshell and dump to disk. In any space, we can 
use the dumpfiles plugin to get a copy of the cached executable file from disk. 
I think the current state of the plugins is sufficient -- they do their best 
without acting in an unexpected way, and they report when attempts to dump wind 
up failing (alerting the user to investigate manually).

Original comment by michael.hale@gmail.com on 7 Mar 2014 at 8:01

Changed state: Fixed

ksanchezcld / volatility

SanityCheckException, --unsafe, refactor of PE dumping plugins #291