Potential issue with impscan.enum_apis and export forwarding

[GoogleCodeExporter]

This issue concerns impscan.enum_apis.

impscan.enum_apis iterates over the given modules using the 
_LDR_DATA_TABLE_ENTRY exports method. This method in turn uses the 
_exported_functions method of the _IMAGE_EXPORT_DIRECTORY object.

_exported_functions handles export forwarded functions by returning a None 
value for the address and setting the function name to an C-string matching 
"ForwardLibrary.ForwardName".

The exports method dutifully respects this behaviour.

impscan.enum_apis adds this None address (presumably interpreted as -1?) to 
mod.DllBase to deliver -1 as an incorrect result (as the NoneObject acts 
strictly with arithmetic operators).

impscan.enum_apis can't be expected to have enough data to resolve such an 
issue (it only really knows about the all_mods modules).

Is returning -1 the best we can do here? Perhaps we should add in either more 
error information to the returned value or to the impscan.enum_apis 
documentation so that this behaviour is flagged to any potential users?

Original issue reported on code.google.com by carl.pulley on 6 Apr 2013 at 9:48

[GoogleCodeExporter]

Here's a potential patch for the impscan.enum_apis function (basically, we do 
what we can with the information that is to hand):

  def enum_apis(all_mods):
    exports = {}
    named_modules = dict( (".".join(str(mod.BaseDllName).split(".")[:-1]).lower(), mod) for mod in all_mods )

    # FIXME: can forwarded exports be chained?
    # FIXME: can forwarded function names be ordinals?
    for mod in all_mods:
      for ordinal, func_addr, func_name in mod.exports():
        if isinstance(func_addr, obj.NoneObject) and "function is forwarded" in func_addr.reason:
          # we are dealing with a forwarded export
          forwarded_parts = str(func_name).split(".")
          forwarded_mod_name = forwarded_parts[0].lower()
          forwarded_func_name = ".".join(forwarded_parts[1:])
          if forwarded_mod_name not in named_modules:
            debug.warning("function forwarded to an unknown module [module: {0} -forward-> module: {1}; function: {2}]".format(str(mod.BaseDllName), forwarded_mod_name, forwarded_func_name))
            continue
          forwarded_mod = named_modules[forwarded_mod_name]
          forwarded_func_addr = forwarded_mod.getprocaddress(forwarded_func_name)
          if forwarded_func_addr is None:
            debug.warning("null function address [module: {0} -forward-> module: {1}; function: {2}]".format(str(mod.BaseDllName), forwarded_mod_name, forwarded_func_name))
            continue
          exports[int(forwarded_mod.DllBase + forwarded_func_addr)] = (forwarded_mod, forwarded_func_name)
        else:
          name = func_name or ordinal or ''
          exports[int(mod.DllBase + func_addr)] = (mod, str(name))

Original comment by carl.pulley on 7 Apr 2013 at 8:27

[GoogleCodeExporter]

I've just been profiling this code and noticed that it is quite inefficient.

Calling exports on a module is not cheap and so should be minimised.

As getprocaddress calls exports to perform its search, and we're within an 
exports based loop, we're getting quite a performance hit here.

Original comment by carl.pulley on 7 Apr 2013 at 4:43

[GoogleCodeExporter]

Original comment by jamie.l...@gmail.com on 7 Apr 2013 at 4:44

[GoogleCodeExporter]

Following code provides us with an approx. 50% speedup:

  def enum_apis(all_mods):
    def get_forwarded_func(func_name, mod, mod_func_dict):
      forwarded_parts = str(func_name).split(".")
      forwarded_mod_name = forwarded_parts[0].lower()
      forwarded_func_name = ".".join(forwarded_parts[1:])
      if forwarded_mod_name not in mod_func_dict:
        debug.warning("function forwarded to an unknown module [{0} -forward-> {1}.{2}]".format(str(mod.BaseDllName), forwarded_mod_name, forwarded_func_name))
        raise ValueError()
      if forwarded_func_name not in mod_func_dict[forwarded_mod_name]:
        debug.warning("forwarded to an unknown function [{0} -forward-> {1}.{2}]".format(str(mod.BaseDllName), forwarded_mod_name, forwarded_func_name))
        raise ValueError()
      forwarded_func_addr = mod_func_dict[forwarded_mod_name][forwarded_func_name]
      return (forwarded_func_addr, forwarded_func_name, forwarded_mod)

    def dllname(mod):
      return ".".join(str(mod.BaseDllName).split(".")[:-1]).lower()

    # If function has no name (i.e. ordinal only), then name (nm) will be an obj.NoneObject instance
    exports = {}
    lookup_table = dict( (dllname(mod), dict( (str(nm or o), rva) for o, rva, nm in mod.exports() )) for mod in all_mods )

    # FIXME: can forwarded exports be chained?
    # FIXME: can forwarded function names be ordinals?
    for mod in all_mods:
      for ordinal, func_addr, func_name in mod.exports():
        if isinstance(func_addr, obj.NoneObject) and "function is forwarded" in func_addr.reason:
          # we are dealing with a forwarded export
          try:
            forwarded_func_addr, forwarded_func_name, forwarded_mod = get_forwarded_func(func_name, mod, lookup_table)
            exports[int(forwarded_mod.DllBase + forwarded_func_addr)] = (forwarded_mod, forwarded_func_name)
          except ValueError:
            continue
        else:
          name = func_name or ordinal or ''
          exports[int(mod.DllBase + func_addr)] = (mod, str(name))

Original comment by carl.pulley on 7 Apr 2013 at 6:16

[GoogleCodeExporter]

Thinking about things further, here's a much nicer and more efficient 
"solution":

   def enum_apis(all_mods):
    exports = {}
    lookup_table = {}
    forwarded_funcs = []

    def get_func_addr(func_name, mod, mod_func_dict):
      if mod not in mod_func_dict:
        raise ValueError("function forwarded to an unknown module [-forward-> {0}.{1}]".format(mod, func_name))
      if func_name not in mod_func_dict[mod]:
        raise ValueError("forwarded to an unknown function [-forward-> {0}.{1}]".format(mod, func_name))
      return mod_func_dict[mod][func_name]

    def dllname(mod):
      return ".".join(str(mod.BaseDllName).split(".")[:-1]).lower()

    # FIXME: can forwarded exports be chained?
    # FIXME: can forwarded function names be ordinals?
    for mod in all_mods:
      for ordinal, func_addr, func_name in mod.exports():
        mod_name = dllname(mod)
        if mod_name not in lookup_table:
          lookup_table[mod_name] = {}
        lookup_table[mod_name][str(func_name or ordinal)] = (mod, func_addr)
        if isinstance(func_addr, obj.NoneObject) and "function is forwarded" in func_addr.reason:
          # we are dealing with a forwarded export - stack information and process latter
          forwarded_parts = str(func_name).split(".")
          forwarded_mod_name = forwarded_parts[0].lower()
          forwarded_func_name = ".".join(forwarded_parts[1:])
          forwarded_funcs += [ (forwarded_mod_name, forwarded_func_name) ]
        else:
          name = func_name or ordinal or ''
          exports[int(mod.DllBase + func_addr)] = (mod, str(name))

    # As lookup_table is now built, process all pending export forwarded functions
    for mod_nm, func_name in forwarded_funcs:
      try:
        mod, func_addr = get_func_addr(func_name, mod_nm, lookup_table)
        exports[int(mod.DllBase + func_addr)] = (mod, func_name)
      except ValueError, exn:
        debug.warning(str(exn))

Original comment by carl.pulley on 7 Apr 2013 at 11:21

[GoogleCodeExporter]

Hey Carl, supplying patches with your bug reports is _greatly_ appreciated. 
I'll be taking a look Monday/Tuesday and will get back to you. Thanks again!

Original comment by michael.hale@gmail.com on 7 Apr 2013 at 11:24

[GoogleCodeExporter]

Hey Carl, 

Alright I looked over this today. Just to make sure I understand the issue, my 
main two take-aways are:

1) we're incorrectly adding int(mod.DllBase + func_addr) to the exports 
dictionary in enum_apis when func_addr can sometimes be a NoneObject (which is 
obviously not good)
2) there is insufficient debugging information to let users know about 
forwarded exports 

So I think both of these issues can be addressed using the attached patch that 
1) creates a more descriptive NoneObject reason and 2) checks if func_addr is a 
NoneObject before adding it to the exports dictionary. 

Now that we create the NoneObjects with more descriptive reasons, we can use -d 
-d -d to access the debugging messages. Anytime a NoneObject() is instantiated, 
the reason will be printed to the debug facility. 

$ cp contrib/plugins/enumfunc.py volatility/plugins/
$ python vol.py -f FILE --profile PROFILE enumfunc --process-only --export-only 
-d -d -d

Process              Type       Module               Ordinal    Address         
     Name
[snip] 
DEBUG1  : volatility.obj      : None object instantiated: Ordinal function 0 in 
module kernel32.dll forwards to NTDLL.RtlAcquireSRWLockExclusive
svchost.exe          Export     kernel32.dll         1          
0x0000000000000000 NTDLL.RtlAcquireSRWLockExclusive
DEBUG1  : volatility.obj      : None object instantiated: Ordinal function 1 in 
module kernel32.dll forwards to NTDLL.RtlAcquireSRWLockShared
svchost.exe          Export     kernel32.dll         2          
0x0000000000000000 NTDLL.RtlAcquireSRWLockShared
svchost.exe          Export     kernel32.dll         3          
0x0000000076b244b0 ActivateActCtx
svchost.exe          Export     kernel32.dll         4          
0x0000000076b86b20 AddAtomA
svchost.exe          Export     kernel32.dll         5          
0x0000000076b86ac0 AddAtomW
svchost.exe          Export     kernel32.dll         6          
0x0000000076b8ad90 AddConsoleAliasA
svchost.exe          Export     kernel32.dll         7          
0x0000000076b8ae00 AddConsoleAliasW
[snip]

Note that _LDR_DATA_TABLE_ENTRY.exports() leaves it up to the caller what to do 
with forwarded exports. In the case of the enumfunc plugin, it prints 0 in the 
Address column and forwarded name string (i.e. NTDLL.RtlAcquireSRWLockShared) 
in the Name column. However, the ImpScan.enum_apis() method, after the patch, 
just skips them. But please note that by skipping them, we don't end up with 
less entries in the exports dictionary that ImpScan.enum_apis() returns. For 
example, if a.dll exports a function that is forwarded to b.ExampleFunc, the 
enum_apis() method will still have an entry for b.ExampleFunc at address XYZ 
because it will have parsed the export table of b.dll. 

And regardless of what the caller does with forwarded exports, since the 
NoneObject is instantiated within the "private" 
_IMAGE_EXPORT_DIRECTORY._exported_functions(), the debug facility (-d -d -d) 
will show the debug messages (as opposed to every caller needing to check if 
func_addr is a NoneObject and then printing the reason). 

Does that make sense? By reading over your replacement enum_apis(), I feel like 
you're expecting the exports dictionary to have more entries than the original 
method, but that's not the case (at least in my testing and in theory). Can you 
look over and test the patch and let me know what you think? 

Thanks!

Original comment by michael.hale@gmail.com on 8 Apr 2013 at 10:58

Attachments:

• forwarded_exports.patch

[GoogleCodeExporter]

The patch looks great thanks.

I think you're right - I may be expecting too much from enum_apis?

I was attempting to follow the forwarded function so that enum_apis returned a 
valid address for that forwarded function. It definitely feels better leaving 
that type of alias resolution to the calling code.

Original comment by carl.pulley on 9 Apr 2013 at 2:45

[GoogleCodeExporter]

Hey Carl - thanks for reviewing and commenting. 

> I was attempting to follow the forwarded function so that enum_apis 
> returned a valid address for that forwarded function

Yeah, I see what you mean. Hopefully it made sense what I said about the 
exports dictionary containing an entry for the forwarded function, even if we 
don't explicitly "follow" it (since we end up parsing the export table of all 
modules in the space anyway). So in the end, with enum_apis(), we still have an 
exports dictionary with the addresses of all *destination* APIs (i.e. where the 
assembly instructions actually exist) but we just provide debug messages for 
the *source* alias. 

I'll go ahead and commit the patch!

Original comment by michael.hale@gmail.com on 9 Apr 2013 at 2:58

[GoogleCodeExporter]

This issue was closed by revision r3358.

Original comment by michael.hale@gmail.com on 9 Apr 2013 at 2:59

• Changed state: Fixed